[Update] 控制组织管理员不允许更新、删除超级用户；修复ViewSet API批量更新的bug (#2629)

* [Update] 控制组织管理员不允许编辑（更新、删除）超级用户 - 待续（控制批量更新API）

* [Update] 修改方法名称

* [Update] 控制组织管理员不允许批量更新包含超级用户的用户列表

* [Bugfix] 修复所有ViewSet API进行批量更新时rest_framework_bulk库内部的bug

* [Update] 修改 OpenID Middleware 日志输出模式 info => debug

apps/assets/serializers/admin_user.py

@@ -3,6 +3,8 @@
 from django.core.cache import cache
 from rest_framework import serializers
 
+from common.serializers import AdaptedBulkListSerializer
+
 from ..models import Node, AdminUser
 from ..const import ADMIN_USER_CONN_CACHE_KEY
 
@@ -18,6 +20,7 @@ class AdminUserSerializer(serializers.ModelSerializer):
     reachable_amount = serializers.SerializerMethodField()
 
     class Meta:
+        list_serializer_class = AdaptedBulkListSerializer
         model = AdminUser
         fields = '__all__'
 

apps/assets/serializers/asset.py

@@ -1,9 +1,9 @@
 # -*- coding: utf-8 -*-
 #
 from rest_framework import serializers
-from rest_framework_bulk.serializers import BulkListSerializer
 
 from common.mixins import BulkSerializerMixin
+from common.serializers import AdaptedBulkListSerializer
 from ..models import Asset
 from .system_user import AssetSystemUserSerializer
 
@@ -19,7 +19,7 @@ class AssetSerializer(BulkSerializerMixin, serializers.ModelSerializer):
     """
     class Meta:
         model = Asset
-        list_serializer_class = BulkListSerializer
+        list_serializer_class = AdaptedBulkListSerializer
         fields = '__all__'
         validators = []
 

apps/assets/serializers/cmd_filter.py

@@ -3,6 +3,7 @@
 from rest_framework import serializers
 
 from common.fields import ChoiceDisplayField
+from common.serializers import AdaptedBulkListSerializer
 from ..models import CommandFilter, CommandFilterRule, SystemUser
 
 
@@ -12,6 +13,7 @@ class CommandFilterSerializer(serializers.ModelSerializer):
 
     class Meta:
         model = CommandFilter
+        list_serializer_class = AdaptedBulkListSerializer
         fields = '__all__'
 
 
@@ -21,3 +23,4 @@ class CommandFilterRuleSerializer(serializers.ModelSerializer):
     class Meta:
         model = CommandFilterRule
         fields = '__all__'
+        list_serializer_class = AdaptedBulkListSerializer

apps/assets/serializers/domain.py

@@ -2,6 +2,8 @@
 #
 from rest_framework import serializers
 
+from common.serializers import AdaptedBulkListSerializer
+
 from ..models import Domain, Gateway
 
 
@@ -12,6 +14,7 @@ class DomainSerializer(serializers.ModelSerializer):
     class Meta:
         model = Domain
         fields = '__all__'
+        list_serializer_class = AdaptedBulkListSerializer
 
     @staticmethod
     def get_asset_count(obj):
@@ -25,6 +28,7 @@ class DomainSerializer(serializers.ModelSerializer):
 class GatewaySerializer(serializers.ModelSerializer):
     class Meta:
         model = Gateway
+        list_serializer_class = AdaptedBulkListSerializer
         fields = [
             'id', 'name', 'ip', 'port', 'protocol', 'username',
             'domain', 'is_active', 'date_created', 'date_updated',

apps/assets/serializers/label.py

@@ -1,7 +1,8 @@
 # -*- coding: utf-8 -*-
 #
 from rest_framework import serializers
-from rest_framework_bulk.serializers import BulkListSerializer
+
+from common.serializers import AdaptedBulkListSerializer
 
 from ..models import Label
 
@@ -12,7 +13,7 @@ class LabelSerializer(serializers.ModelSerializer):
     class Meta:
         model = Label
         fields = '__all__'
-        list_serializer_class = BulkListSerializer
+        list_serializer_class = AdaptedBulkListSerializer
 
     @staticmethod
     def get_asset_count(obj):

apps/assets/serializers/system_user.py

@@ -1,5 +1,7 @@
 from rest_framework import serializers
 
+from common.serializers import AdaptedBulkListSerializer
+
 from ..models import SystemUser, Asset
 from .base import AuthSerializer
 
@@ -17,6 +19,7 @@ class SystemUserSerializer(serializers.ModelSerializer):
     class Meta:
         model = SystemUser
         exclude = ('_password', '_private_key', '_public_key')
+        list_serializer_class = AdaptedBulkListSerializer
 
     def get_field_names(self, declared_fields, info):
         fields = super(SystemUserSerializer, self).get_field_names(declared_fields, info)

apps/authentication/backends/openid/middleware.py

@@ -23,15 +23,15 @@ class OpenIDAuthenticationMiddleware(MiddlewareMixin):
     def process_request(self, request):
         # Don't need openid auth if AUTH_OPENID is False
         if not settings.AUTH_OPENID:
-            logger.info("Not settings.AUTH_OPENID")
+            logger.debug("Not settings.AUTH_OPENID")
             return
         # Don't need check single logout if user not authenticated
         if not request.user.is_authenticated:
-            logger.info("User is not authenticated")
+            logger.debug("User is not authenticated")
             return
         elif not request.session[BACKEND_SESSION_KEY].endswith(
                 BACKEND_OPENID_AUTH_CODE):
-            logger.info("BACKEND_SESSION_KEY is not BACKEND_OPENID_AUTH_CODE")
+            logger.debug("BACKEND_SESSION_KEY is not BACKEND_OPENID_AUTH_CODE")
             return
 
         # Check openid user single logout or not with access_token
@@ -40,7 +40,6 @@ class OpenIDAuthenticationMiddleware(MiddlewareMixin):
             client.openid_connect_client.userinfo(
                 token=request.session.get(OIDT_ACCESS_TOKEN)
             )
-
         except Exception as e:
             logout(request)
             logger.error(e)

apps/common/mixins.py

@@ -4,6 +4,10 @@ from django.db import models
 from django.http import JsonResponse
 from django.utils import timezone
 from django.utils.translation import ugettext_lazy as _
+from rest_framework.utils import html
+from rest_framework.settings import api_settings
+from rest_framework.exceptions import ValidationError
+from rest_framework.fields import SkipField
 
 
 class NoDeleteQuerySet(models.query.QuerySet):
@@ -89,6 +93,60 @@ class BulkSerializerMixin(object):
         return ret
 
 
+class BulkListSerializerMixin(object):
+    """
+    Become rest_framework_bulk doing bulk update raise Exception:
+    'QuerySet' object has no attribute 'pk' when doing bulk update
+    so rewrite it .
+    https://github.com/miki725/django-rest-framework-bulk/issues/68
+    """
+
+    def to_internal_value(self, data):
+        """
+        List of dicts of native values <- List of dicts of primitive datatypes.
+        """
+        if html.is_html_input(data):
+            data = html.parse_html_list(data)
+
+        if not isinstance(data, list):
+            message = self.error_messages['not_a_list'].format(
+                input_type=type(data).__name__
+            )
+            raise ValidationError({
+                api_settings.NON_FIELD_ERRORS_KEY: [message]
+            }, code='not_a_list')
+
+        if not self.allow_empty and len(data) == 0:
+            if self.parent and self.partial:
+                raise SkipField()
+
+            message = self.error_messages['empty']
+            raise ValidationError({
+                api_settings.NON_FIELD_ERRORS_KEY: [message]
+            }, code='empty')
+
+        ret = []
+        errors = []
+
+        for item in data:
+            try:
+                # prepare child serializer to only handle one instance
+                self.child.instance = self.instance.get(id=item['id']) if self.instance else None
+                self.child.initial_data = item
+                # raw
+                validated = self.child.run_validation(item)
+            except ValidationError as exc:
+                errors.append(exc.detail)
+            else:
+                ret.append(validated)
+                errors.append({})
+
+        if any(errors):
+            raise ValidationError(errors)
+
+        return ret
+
+
 class DatetimeSearchMixin:
     date_format = '%Y-%m-%d'
     date_from = date_to = None

apps/common/serializers.py

@@ -0,0 +1,9 @@
+# -*- coding: utf-8 -*-
+#
+
+from .mixins import BulkListSerializerMixin
+from rest_framework_bulk.serializers import BulkListSerializer
+
+
+class AdaptedBulkListSerializer(BulkListSerializerMixin, BulkListSerializer):
+    pass

apps/orgs/serializers.py

@@ -1,11 +1,11 @@
 
 from rest_framework.serializers import ModelSerializer
 from rest_framework import serializers
-from rest_framework_bulk import BulkListSerializer
 
 from users.models import User, UserGroup
 from assets.models import Asset, Domain, AdminUser, SystemUser, Label
 from perms.models import AssetPermission
+from common.serializers import AdaptedBulkListSerializer
 from .utils import set_current_org, get_current_org
 from .models import Organization
 from .mixins import OrgMembershipSerializerMixin
@@ -14,7 +14,7 @@ from .mixins import OrgMembershipSerializerMixin
 class OrgSerializer(ModelSerializer):
     class Meta:
         model = Organization
-        list_serializer_class = BulkListSerializer
+        list_serializer_class = AdaptedBulkListSerializer
         fields = '__all__'
         read_only_fields = ['created_by', 'date_created']
 
@@ -70,12 +70,12 @@ class OrgReadSerializer(ModelSerializer):
 class OrgMembershipAdminSerializer(OrgMembershipSerializerMixin, ModelSerializer):
     class Meta:
         model = Organization.admins.through
-        list_serializer_class = BulkListSerializer
+        list_serializer_class = AdaptedBulkListSerializer
         fields = '__all__'
 
 
 class OrgMembershipUserSerializer(OrgMembershipSerializerMixin, ModelSerializer):
     class Meta:
         model = Organization.users.through
-        list_serializer_class = BulkListSerializer
+        list_serializer_class = AdaptedBulkListSerializer
         fields = '__all__'

apps/terminal/serializers/v1.py

@@ -1,9 +1,9 @@
 # -*- coding: utf-8 -*-
 #
 from rest_framework import serializers
-from rest_framework_bulk.serializers import BulkListSerializer
 
 from common.mixins import BulkSerializerMixin
+from common.serializers import AdaptedBulkListSerializer
 from ..models import Terminal, Status, Session, Task
 
 
@@ -29,7 +29,7 @@ class SessionSerializer(BulkSerializerMixin, serializers.ModelSerializer):
 
     class Meta:
         model = Session
-        list_serializer_class = BulkListSerializer
+        list_serializer_class = AdaptedBulkListSerializer
         fields = '__all__'
 
 
@@ -44,7 +44,7 @@ class TaskSerializer(BulkSerializerMixin, serializers.ModelSerializer):
     class Meta:
         fields = '__all__'
         model = Task
-        list_serializer_class = BulkListSerializer
+        list_serializer_class = AdaptedBulkListSerializer
 
 
 class ReplaySerializer(serializers.Serializer):

apps/users/api/user.py

@@ -5,6 +5,7 @@ from django.core.cache import cache
 from django.contrib.auth import logout
 from django.utils.translation import ugettext as _
 
+from rest_framework import status
 from rest_framework import generics
 from rest_framework.response import Response
 from rest_framework.permissions import IsAuthenticated
@@ -52,9 +53,72 @@ class UserViewSet(IDInFilterMixin, BulkModelViewSet):
             self.permission_classes = (IsOrgAdminOrAppUser,)
         return super().get_permissions()
 
+    def _deny_permission(self, instance):
+        """
+        check current user has permission to handle instance
+        (update, destroy, bulk_update, bulk destroy)
+        """
+        return not self.request.user.is_superuser and instance.is_superuser
+
+    def destroy(self, request, *args, **kwargs):
+        """
+        rewrite because limit org_admin destroy superuser
+        """
+        instance = self.get_object()
+        if self._deny_permission(instance):
+            data = {'msg': _("You do not have permission.")}
+            return Response(data=data, status=status.HTTP_403_FORBIDDEN)
+
+        return super().destroy(request, *args, **kwargs)
+
+    def update(self, request, *args, **kwargs):
+        """
+        rewrite because limit org_admin update superuser
+        """
+        instance = self.get_object()
+        if self._deny_permission(instance):
+            data = {'msg': _("You do not have permission.")}
+            return Response(data=data, status=status.HTTP_403_FORBIDDEN)
+
+        return super().update(request, *args, **kwargs)
+
+    def _bulk_deny_permission(self, instances):
+        deny_instances = [i for i in instances if self._deny_permission(i)]
+        if len(deny_instances) > 0:
+            return True
+        else:
+            return False
+
     def allow_bulk_destroy(self, qs, filtered):
+        if self._bulk_deny_permission(filtered):
+            return False
         return qs.count() != filtered.count()
 
+    def bulk_update(self, request, *args, **kwargs):
+        """
+        rewrite because limit org_admin update superuser
+        """
+        partial = kwargs.pop('partial', False)
+
+        # restrict the update to the filtered queryset
+        queryset = self.filter_queryset(self.get_queryset())
+        if self._bulk_deny_permission(queryset):
+            data = {'msg': _("You do not have permission.")}
+            return Response(data=data, status=status.HTTP_403_FORBIDDEN)
+
+        serializer = self.get_serializer(
+            queryset, data=request.data, many=True, partial=partial,
+        )
+
+        try:
+            serializer.is_valid(raise_exception=True)
+        except Exception as e:
+            data = {'error': str(e)}
+            return Response(data=data, status=status.HTTP_400_BAD_REQUEST)
+
+        self.perform_bulk_update(serializer)
+        return Response(serializer.data, status=status.HTTP_200_OK)
+
 
 class UserChangePasswordApi(generics.RetrieveUpdateAPIView):
     permission_classes = (IsOrgAdmin,)

apps/users/serializers/v1.py

@@ -3,10 +3,10 @@
 from django.utils.translation import ugettext_lazy as _
 
 from rest_framework import serializers
-from rest_framework_bulk import BulkListSerializer
 
 from common.utils import get_signer, validate_ssh_public_key
 from common.mixins import BulkSerializerMixin
+from common.serializers import AdaptedBulkListSerializer
 from ..models import User, UserGroup
 
 signer = get_signer()
@@ -16,7 +16,7 @@ class UserSerializer(BulkSerializerMixin, serializers.ModelSerializer):
 
     class Meta:
         model = User
-        list_serializer_class = BulkListSerializer
+        list_serializer_class = AdaptedBulkListSerializer
         fields = [
             'id', 'name', 'username', 'email', 'groups', 'groups_display',
             'role', 'role_display', 'avatar_url', 'wechat', 'phone',
@@ -52,7 +52,7 @@ class UserGroupSerializer(BulkSerializerMixin, serializers.ModelSerializer):
 
     class Meta:
         model = UserGroup
-        list_serializer_class = BulkListSerializer
+        list_serializer_class = AdaptedBulkListSerializer
         fields = '__all__'
         read_only_fields = ['created_by']
 

apps/users/templates/users/user_detail.html

@@ -22,11 +22,11 @@
                                 <a href="{% url 'users:user-granted-asset' pk=user_object.id %}" class="text-center"><i class="fa fa-cubes"></i> {% trans 'Asset granted' %}</a>
                             </li>
                             <li class="pull-right">
-                                <a class="btn btn-outline btn-default" href="{% url 'users:user-update' pk=user_object.id %}"><i class="fa fa-edit"></i>{% trans 'Update' %}</a>
+                                <a class="btn btn-outline {% if user_object.is_superuser and not request.user.is_superuser %} disabled {% else %} btn-default {% endif %}" href="{% url 'users:user-update' pk=user_object.id %}"><i class="fa fa-edit"></i>{% trans 'Update' %}</a>
                             </li>
 
                             <li class="pull-right">
-                                <a class="btn btn-outline {% if request.user != user_object and user_object.username != "admin" %} btn-danger btn-delete-user {% else %} disabled {% endif %}">
+                                <a class="btn btn-outline {% if request.user == user_object or user_object.username == "admin" or user_object.is_superuser and not request.user.is_superuser %} disabled  {% else %} btn-danger btn-delete-user {% endif %}">
                                     <i class="fa fa-trash-o"></i>{% trans 'Delete' %}
                                 </a>
                             </li>

apps/users/templates/users/user_list.html

@@ -77,10 +77,16 @@ function initTable() {
                 }
              }},
             {targets: 7, createdCell: function (td, cellData, rowData) {
-                var update_btn = '<a href="{% url "users:user-update" pk=DEFAULT_PK %}" class="btn btn-xs btn-info">{% trans "Update" %}</a>'.replace('00000000-0000-0000-0000-000000000000', cellData);
+                var update_btn = "";
+                if (rowData.role === 'Admin' && ('{{ request.user.role }}' !== 'Admin')) {
+                    update_btn = '<a class="btn btn-xs disabled btn-info">{% trans "Update" %}</a>';
+                }
+                else{
+                    update_btn = '<a href="{% url "users:user-update" pk=DEFAULT_PK %}" class="btn btn-xs btn-info">{% trans "Update" %}</a>'.replace('00000000-0000-0000-0000-000000000000', cellData);
+                }
 
                 var del_btn = "";
-                if (rowData.id === 1 || rowData.username === "admin" || rowData.username === "{{ request.user.username }}") {
+                if (rowData.id === 1 || rowData.username === "admin" || rowData.username === "{{ request.user.username }}" || (rowData.role === 'Admin' && ('{{ request.user.role }}' !== 'Admin'))) {
                     del_btn = '<a class="btn btn-xs btn-danger m-l-xs" disabled>{% trans "Delete" %}</a>'
                             .replace('{{ DEFAULT_PK }}', cellData)
                             .replace('99991938', rowData.name);

apps/users/views/user.py

@@ -107,6 +107,15 @@ class UserUpdateView(AdminUserRequiredMixin, SuccessMessageMixin, UpdateView):
     success_url = reverse_lazy('users:user-list')
     success_message = update_success_msg
 
+    def _deny_permission(self):
+        obj = self.get_object()
+        return not self.request.user.is_superuser and obj.is_superuser
+
+    def get(self, request, *args, **kwargs):
+        if self._deny_permission():
+            return redirect(self.success_url)
+        return super().get(request, *args, **kwargs)
+
     def get_context_data(self, **kwargs):
         check_rules = get_password_check_rules()
         context = {