From caa5060ecd5bf2466cdc272473c94e61d17b09f7 Mon Sep 17 00:00:00 2001 From: BaiJiangJie <32935519+BaiJiangJie@users.noreply.github.com> Date: Thu, 25 Apr 2019 10:11:50 +0800 Subject: [PATCH] =?UTF-8?q?[Update]=20=E6=8E=A7=E5=88=B6=E7=BB=84=E7=BB=87?= =?UTF-8?q?=E7=AE=A1=E7=90=86=E5=91=98=E4=B8=8D=E5=85=81=E8=AE=B8=E6=9B=B4?= =?UTF-8?q?=E6=96=B0=E3=80=81=E5=88=A0=E9=99=A4=E8=B6=85=E7=BA=A7=E7=94=A8?= =?UTF-8?q?=E6=88=B7=EF=BC=9B=E4=BF=AE=E5=A4=8DViewSet=20API=E6=89=B9?= =?UTF-8?q?=E9=87=8F=E6=9B=B4=E6=96=B0=E7=9A=84bug=20(#2629)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Update] 控制组织管理员不允许编辑（更新、删除）超级用户 - 待续（控制批量更新API） * [Update] 修改方法名称 * [Update] 控制组织管理员不允许批量更新包含超级用户的用户列表 * [Bugfix] 修复所有ViewSet API进行批量更新时rest_framework_bulk库内部的bug * [Update] 修改 OpenID Middleware 日志输出模式 info => debug --- apps/assets/serializers/admin_user.py | 3 + apps/assets/serializers/asset.py | 4 +- apps/assets/serializers/cmd_filter.py | 3 + apps/assets/serializers/domain.py | 4 ++ apps/assets/serializers/label.py | 5 +- apps/assets/serializers/system_user.py | 3 + .../backends/openid/middleware.py | 7 +- apps/common/mixins.py | 58 +++++++++++++++++ apps/common/serializers.py | 9 +++ apps/orgs/serializers.py | 8 +-- apps/terminal/serializers/v1.py | 6 +- apps/users/api/user.py | 64 +++++++++++++++++++ apps/users/serializers/v1.py | 6 +- apps/users/templates/users/user_detail.html | 4 +- apps/users/templates/users/user_list.html | 10 ++- apps/users/views/user.py | 9 +++ 16 files changed, 181 insertions(+), 22 deletions(-) create mode 100644 apps/common/serializers.py diff --git a/apps/assets/serializers/admin_user.py b/apps/assets/serializers/admin_user.py index 009caa1ce..e44679995 100644 --- a/apps/assets/serializers/admin_user.py +++ b/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__' diff --git a/apps/assets/serializers/asset.py b/apps/assets/serializers/asset.py index 9640aff7f..c0f435adc 100644 --- a/apps/assets/serializers/asset.py +++ b/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 = [] diff --git a/apps/assets/serializers/cmd_filter.py b/apps/assets/serializers/cmd_filter.py index e49f89bcf..26040f6aa 100644 --- a/apps/assets/serializers/cmd_filter.py +++ b/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 diff --git a/apps/assets/serializers/domain.py b/apps/assets/serializers/domain.py index 9cddf0c49..553911eb8 100644 --- a/apps/assets/serializers/domain.py +++ b/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', diff --git a/apps/assets/serializers/label.py b/apps/assets/serializers/label.py index b45e1e508..9fbc9e804 100644 --- a/apps/assets/serializers/label.py +++ b/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): diff --git a/apps/assets/serializers/system_user.py b/apps/assets/serializers/system_user.py index b4387d1ff..c737f8cbe 100644 --- a/apps/assets/serializers/system_user.py +++ b/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) diff --git a/apps/authentication/backends/openid/middleware.py b/apps/authentication/backends/openid/middleware.py index 75a752708..b1556ff04 100644 --- a/apps/authentication/backends/openid/middleware.py +++ b/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) diff --git a/apps/common/mixins.py b/apps/common/mixins.py index 0a7d15fef..a5e9a58d3 100644 --- a/apps/common/mixins.py +++ b/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 diff --git a/apps/common/serializers.py b/apps/common/serializers.py new file mode 100644 index 000000000..7f2abce92 --- /dev/null +++ b/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 diff --git a/apps/orgs/serializers.py b/apps/orgs/serializers.py index db3b56171..1c93cbf91 100644 --- a/apps/orgs/serializers.py +++ b/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__' diff --git a/apps/terminal/serializers/v1.py b/apps/terminal/serializers/v1.py index adf75c936..27b12b77a 100644 --- a/apps/terminal/serializers/v1.py +++ b/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): diff --git a/apps/users/api/user.py b/apps/users/api/user.py index 33874deb7..63c3dab12 100644 --- a/apps/users/api/user.py +++ b/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,) diff --git a/apps/users/serializers/v1.py b/apps/users/serializers/v1.py index 160df7c62..b8c91417d 100644 --- a/apps/users/serializers/v1.py +++ b/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'] diff --git a/apps/users/templates/users/user_detail.html b/apps/users/templates/users/user_detail.html index deb96eb68..351823a9e 100644 --- a/apps/users/templates/users/user_detail.html +++ b/apps/users/templates/users/user_detail.html @@ -22,11 +22,11 @@ {% trans 'Asset granted' %}
  • - {% trans 'Update' %} + {% trans 'Update' %}
  • - + {% trans 'Delete' %}
    • diff --git a/apps/users/templates/users/user_list.html b/apps/users/templates/users/user_list.html index 2ebfa6664..f3e2b7894 100644 --- a/apps/users/templates/users/user_list.html +++ b/apps/users/templates/users/user_list.html @@ -77,10 +77,16 @@ function initTable() { } }}, {targets: 7, createdCell: function (td, cellData, rowData) { - var update_btn = '{% trans "Update" %}'.replace('00000000-0000-0000-0000-000000000000', cellData); + var update_btn = ""; + if (rowData.role === 'Admin' && ('{{ request.user.role }}' !== 'Admin')) { + update_btn = '{% trans "Update" %}'; + } + else{ + update_btn = '{% trans "Update" %}'.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 = '{% trans "Delete" %}' .replace('{{ DEFAULT_PK }}', cellData) .replace('99991938', rowData.name); diff --git a/apps/users/views/user.py b/apps/users/views/user.py index 2ffbd4688..270a9a5c9 100644 --- a/apps/users/views/user.py +++ b/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 = {