From d0f79c2df2efddde5ad5163e60118a4dccd39a1c Mon Sep 17 00:00:00 2001 From: ibuler Date: Wed, 13 Sep 2023 17:05:01 +0800 Subject: [PATCH 1/3] =?UTF-8?q?perf:=20=E6=B7=BB=E5=8A=A0=20check=20api=20?= =?UTF-8?q?=E9=81=BF=E5=85=8D=E6=9C=AA=E8=AE=A4=E8=AF=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- apps/accounts/api/account/task.py | 9 +- apps/assets/api/tree.py | 6 +- apps/authentication/api/confirm.py | 2 +- apps/authentication/api/dingtalk.py | 14 +-- apps/authentication/api/feishu.py | 16 +-- apps/authentication/api/mfa.py | 12 ++ apps/authentication/api/sso.py | 24 ++-- apps/authentication/api/wecom.py | 14 +-- apps/common/drf/renders/__init__.py | 3 +- apps/common/management/commands/check_api.py | 119 +++++++++++++++++++ apps/notifications/api/notifications.py | 2 + apps/ops/views.py | 8 +- 12 files changed, 189 insertions(+), 40 deletions(-) create mode 100644 apps/common/management/commands/check_api.py diff --git a/apps/accounts/api/account/task.py b/apps/accounts/api/account/task.py index 697824806..6bbeb94d7 100644 --- a/apps/accounts/api/account/task.py +++ b/apps/accounts/api/account/task.py @@ -9,9 +9,12 @@ __all__ = [ 'AccountsTaskCreateAPI', ] +from rbac.permissions import RBACPermission + class AccountsTaskCreateAPI(CreateAPIView): serializer_class = serializers.AccountTaskSerializer + permission_classes = [RBACPermission] def check_permissions(self, request): act = request.data.get('action') @@ -19,7 +22,9 @@ class AccountsTaskCreateAPI(CreateAPIView): code = 'accounts.push_account' else: code = 'accounts.verify_account' - return request.user.has_perm(code) + has = request.user.has_perm(code) + if not has: + self.permission_denied(request) def perform_create(self, serializer): data = serializer.validated_data @@ -44,6 +49,6 @@ class AccountsTaskCreateAPI(CreateAPIView): def get_exception_handler(self): def handler(e, context): - return Response({"error": str(e)}, status=400) + return Response({"error": str(e)}, status=401) return handler diff --git a/apps/assets/api/tree.py b/apps/assets/api/tree.py index 1970d33ba..762859ca1 100644 --- a/apps/assets/api/tree.py +++ b/apps/assets/api/tree.py @@ -1,14 +1,14 @@ # ~*~ coding: utf-8 ~*~ from django.db.models import Q +from django.utils.translation import gettext_lazy as _ from rest_framework.generics import get_object_or_404 from rest_framework.response import Response -from django.utils.translation import gettext_lazy as _ from assets.locks import NodeAddChildrenLock +from common.exceptions import JMSException from common.tree import TreeNodeSerializer from common.utils import get_logger -from common.exceptions import JMSException from orgs.mixins import generics from orgs.utils import current_org from .mixin import SerializeToTreeNodeMixin @@ -35,8 +35,8 @@ class NodeChildrenApi(generics.ListCreateAPIView): is_initial = False def initial(self, request, *args, **kwargs): + super().initial(request, *args, **kwargs) self.instance = self.get_object() - return super().initial(request, *args, **kwargs) def perform_create(self, serializer): with NodeAddChildrenLock(self.instance): diff --git a/apps/authentication/api/confirm.py b/apps/authentication/api/confirm.py index 0923875a0..3c0e670f0 100644 --- a/apps/authentication/api/confirm.py +++ b/apps/authentication/api/confirm.py @@ -13,7 +13,7 @@ from ..serializers import ConfirmSerializer class ConfirmBindORUNBindOAuth(RetrieveAPIView): - permission_classes = (UserConfirmation.require(ConfirmType.ReLogin),) + permission_classes = (IsValidUser, UserConfirmation.require(ConfirmType.ReLogin),) def retrieve(self, request, *args, **kwargs): return Response('ok') diff --git a/apps/authentication/api/dingtalk.py b/apps/authentication/api/dingtalk.py index ee2bdae2e..c8d9f7ce0 100644 --- a/apps/authentication/api/dingtalk.py +++ b/apps/authentication/api/dingtalk.py @@ -1,13 +1,13 @@ -from rest_framework.views import APIView from rest_framework.request import Request from rest_framework.response import Response +from rest_framework.views import APIView -from users.models import User -from common.utils import get_logger -from common.permissions import UserConfirmation -from common.api import RoleUserMixin, RoleAdminMixin -from authentication.const import ConfirmType from authentication import errors +from authentication.const import ConfirmType +from common.api import RoleUserMixin, RoleAdminMixin +from common.permissions import UserConfirmation, IsValidUser +from common.utils import get_logger +from users.models import User logger = get_logger(__file__) @@ -27,7 +27,7 @@ class DingTalkQRUnBindBase(APIView): class DingTalkQRUnBindForUserApi(RoleUserMixin, DingTalkQRUnBindBase): - permission_classes = (UserConfirmation.require(ConfirmType.ReLogin),) + permission_classes = (IsValidUser, UserConfirmation.require(ConfirmType.ReLogin),) class DingTalkQRUnBindForAdminApi(RoleAdminMixin, DingTalkQRUnBindBase): diff --git a/apps/authentication/api/feishu.py b/apps/authentication/api/feishu.py index 5a6d3721e..148b99e51 100644 --- a/apps/authentication/api/feishu.py +++ b/apps/authentication/api/feishu.py @@ -1,13 +1,13 @@ -from rest_framework.views import APIView from rest_framework.request import Request from rest_framework.response import Response +from rest_framework.views import APIView -from users.models import User -from common.utils import get_logger -from common.permissions import UserConfirmation -from common.api import RoleUserMixin, RoleAdminMixin -from authentication.const import ConfirmType from authentication import errors +from authentication.const import ConfirmType +from common.api import RoleUserMixin, RoleAdminMixin +from common.permissions import UserConfirmation, IsValidUser +from common.utils import get_logger +from users.models import User logger = get_logger(__file__) @@ -27,7 +27,7 @@ class FeiShuQRUnBindBase(APIView): class FeiShuQRUnBindForUserApi(RoleUserMixin, FeiShuQRUnBindBase): - permission_classes = (UserConfirmation.require(ConfirmType.ReLogin),) + permission_classes = (IsValidUser, UserConfirmation.require(ConfirmType.ReLogin),) class FeiShuQRUnBindForAdminApi(RoleAdminMixin, FeiShuQRUnBindBase): @@ -38,7 +38,7 @@ class FeiShuEventSubscriptionCallback(APIView): """ # https://open.feishu.cn/document/ukTMukTMukTM/uUTNz4SN1MjL1UzM """ - permission_classes = () + permission_classes = (IsValidUser,) def post(self, request: Request, *args, **kwargs): return Response(data=request.data) diff --git a/apps/authentication/api/mfa.py b/apps/authentication/api/mfa.py index 3436c2eeb..049cd177a 100644 --- a/apps/authentication/api/mfa.py +++ b/apps/authentication/api/mfa.py @@ -3,6 +3,7 @@ from django.shortcuts import get_object_or_404 from django.utils.translation import gettext as _ +from rest_framework import exceptions from rest_framework.generics import CreateAPIView from rest_framework.permissions import AllowAny from rest_framework.response import Response @@ -13,6 +14,7 @@ from common.utils import get_logger from users.models.user import User from .. import errors from .. import serializers +from ..errors import SessionEmptyError from ..mixins import AuthMixin logger = get_logger(__name__) @@ -56,6 +58,7 @@ class MFASendCodeApi(AuthMixin, CreateAPIView): if not mfa_backend or not mfa_backend.challenge_required: error = _('Current user not support mfa type: {}').format(mfa_type) raise ValidationError({'error': error}) + try: mfa_backend.send_challenge() except Exception as e: @@ -66,6 +69,15 @@ class MFAChallengeVerifyApi(AuthMixin, CreateAPIView): permission_classes = (AllowAny,) serializer_class = serializers.MFAChallengeSerializer + def initial(self, request, *args, **kwargs): + super().initial(request, *args, **kwargs) + try: + user = self.get_user_from_session() + except SessionEmptyError: + user = None + if not user: + raise exceptions.NotAuthenticated() + def perform_create(self, serializer): user = self.get_user_from_session() code = serializer.validated_data.get('code') diff --git a/apps/authentication/api/sso.py b/apps/authentication/api/sso.py index c3b2d688c..0756bad4e 100644 --- a/apps/authentication/api/sso.py +++ b/apps/authentication/api/sso.py @@ -1,26 +1,27 @@ -from uuid import UUID from urllib.parse import urlencode +from uuid import UUID -from django.contrib.auth import login from django.conf import settings +from django.contrib.auth import login from django.http.response import HttpResponseRedirect +from rest_framework import serializers from rest_framework.decorators import action -from rest_framework.response import Response -from rest_framework.request import Request from rest_framework.permissions import AllowAny +from rest_framework.request import Request +from rest_framework.response import Response -from common.utils.timezone import utc_now -from common.const.http import POST, GET from common.api import JMSGenericViewSet -from common.serializers import EmptySerializer +from common.const.http import POST, GET from common.permissions import OnlySuperUser +from common.serializers import EmptySerializer from common.utils import reverse +from common.utils.timezone import utc_now from users.models import User -from ..serializers import SSOTokenSerializer -from ..models import SSOToken +from ..errors import SSOAuthClosed from ..filters import AuthKeyQueryDeclaration from ..mixins import AuthMixin -from ..errors import SSOAuthClosed +from ..models import SSOToken +from ..serializers import SSOTokenSerializer NEXT_URL = 'next' AUTH_KEY = 'authkey' @@ -67,6 +68,9 @@ class SSOViewSet(AuthMixin, JMSGenericViewSet): if not next_url or not next_url.startswith('/'): next_url = reverse('index') + if not authkey: + raise serializers.ValidationError("authkey is required") + try: authkey = UUID(authkey) token = SSOToken.objects.get(authkey=authkey, expired=False) diff --git a/apps/authentication/api/wecom.py b/apps/authentication/api/wecom.py index 2779cc9eb..e704d3d4b 100644 --- a/apps/authentication/api/wecom.py +++ b/apps/authentication/api/wecom.py @@ -1,13 +1,13 @@ -from rest_framework.views import APIView from rest_framework.request import Request from rest_framework.response import Response +from rest_framework.views import APIView -from users.models import User -from common.utils import get_logger -from common.permissions import UserConfirmation -from common.api import RoleUserMixin, RoleAdminMixin -from authentication.const import ConfirmType from authentication import errors +from authentication.const import ConfirmType +from common.api import RoleUserMixin, RoleAdminMixin +from common.permissions import UserConfirmation, IsValidUser +from common.utils import get_logger +from users.models import User logger = get_logger(__file__) @@ -27,7 +27,7 @@ class WeComQRUnBindBase(APIView): class WeComQRUnBindForUserApi(RoleUserMixin, WeComQRUnBindBase): - permission_classes = (UserConfirmation.require(ConfirmType.ReLogin),) + permission_classes = (IsValidUser, UserConfirmation.require(ConfirmType.ReLogin),) class WeComQRUnBindForAdminApi(RoleAdminMixin, WeComQRUnBindBase): diff --git a/apps/common/drf/renders/__init__.py b/apps/common/drf/renders/__init__.py index bbefe8783..5e0120651 100644 --- a/apps/common/drf/renders/__init__.py +++ b/apps/common/drf/renders/__init__.py @@ -8,7 +8,8 @@ class PassthroughRenderer(renderers.BaseRenderer): """ Return data as-is. View should supply a Response. """ - media_type = '' + media_type = 'application/octet-stream' format = '' + def render(self, data, accepted_media_type=None, renderer_context=None): return data diff --git a/apps/common/management/commands/check_api.py b/apps/common/management/commands/check_api.py new file mode 100644 index 000000000..e8fca1482 --- /dev/null +++ b/apps/common/management/commands/check_api.py @@ -0,0 +1,119 @@ +import re + +from django.conf import settings +from django.core.management.base import BaseCommand +from django.test import Client +from django.urls import URLPattern, URLResolver + +from jumpserver.urls import api_v1 + +path_uuid_pattern = re.compile(r'<\w+:\w+>', re.IGNORECASE) +uuid_pattern = re.compile(r'\(\(\?P<.*>[^)]+\)/\)\?', re.IGNORECASE) +uuid2_pattern = re.compile(r'\(\?P<.*>\[\/\.\]\+\)', re.IGNORECASE) +uuid3_pattern = re.compile(r'\(\?P<.*>\[/\.]\+\)') + + +def list_urls(patterns, path=None): + """ recursive """ + if not path: + path = [] + result = [] + for pattern in patterns: + if isinstance(pattern, URLPattern): + result.append(''.join(path) + str(pattern.pattern)) + elif isinstance(pattern, URLResolver): + result += list_urls(pattern.url_patterns, path + [str(pattern.pattern)]) + return result + + +def parse_to_url(url): + uid = '00000000-0000-0000-0000-000000000000' + + url = url.replace('^', '') + url = url.replace('?$', '') + url = url.replace('(?P[a-z0-9]+)', '') + url = url.replace('((?P[/.]{36})/)?', uid + '/') + url = url.replace('(?P[/.]+)', uid) + url = url.replace('\.', '') + url = url.replace('//', '/') + url = url.strip('$') + url = re.sub(path_uuid_pattern, uid, url) + url = re.sub(uuid2_pattern, uid, url) + url = re.sub(uuid_pattern, uid + '/', url) + url = re.sub(uuid3_pattern, uid, url) + url = url.replace('(00000000-0000-0000-0000-000000000000/)?', uid + '/') + return url + + +def get_api_urls(): + urls = [] + api_urls = list_urls(api_v1) + for ourl in api_urls: + url = parse_to_url(ourl) + if 'render-to-json' in url: + continue + url = '/api/v1/' + url + urls.append((url, ourl)) + return set(urls) + + +known_unauth_urls = [ + "/api/v1/authentication/passkeys/auth/", + "/api/v1/prometheus/metrics/", + "/api/v1/authentication/auth/", + "/api/v1/settings/logo/", + "/api/v1/settings/public/open/", + "/api/v1/authentication/passkeys/login/", + "/api/v1/authentication/tokens/", + "/api/v1/authentication/mfa/challenge/", + "/api/v1/authentication/password/reset-code/", + "/api/v1/authentication/login-confirm-ticket/status/", + "/api/v1/authentication/mfa/select/", + "/api/v1/authentication/mfa/send-code/", + "/api/v1/authentication/sso/login/" +] + +known_error_urls = [ + '/api/v1/terminal/terminals/00000000-0000-0000-0000-000000000000/sessions/00000000-0000-0000-0000-000000000000/replay/download/', + '/api/v1/terminal/sessions/00000000-0000-0000-0000-000000000000/replay/download/', +] + +errors = {} + + +class Command(BaseCommand): + help = 'Check api if unauthorized' + + def handle(self, *args, **options): + settings.LOG_LEVEL = 'ERROR' + urls = get_api_urls() + client = Client() + unauth_urls = [] + error_urls = [] + unformat_urls = [] + + for url, ourl in urls: + if '(' in url or '<' in url: + unformat_urls.append([url, ourl]) + continue + + try: + response = client.get(url, follow=True) + if response.status_code != 401: + errors[url] = str(response.status_code) + ' ' + str(ourl) + unauth_urls.append(url) + except Exception as e: + errors[url] = str(e) + error_urls.append(url) + + print("\nNo auth urls:") + for url in set(unauth_urls) - set(known_unauth_urls): + print('"{}", {}'.format(url, errors.get(url, ''))) + + print("\nError urls:") + for url in set(error_urls): + print(url, ': ' + errors.get(url)) + + print("\nUnformat urls:") + for url in unformat_urls: + print(url) diff --git a/apps/notifications/api/notifications.py b/apps/notifications/api/notifications.py index 827cb9d19..9f641ee70 100644 --- a/apps/notifications/api/notifications.py +++ b/apps/notifications/api/notifications.py @@ -96,6 +96,8 @@ def get_all_test_messages(request): import textwrap from ..notifications import Message from django.shortcuts import HttpResponse + if not request.user.is_superuser: + return HttpResponse('没有权限', status=401) msgs_cls = Message.get_all_sub_messages() html_data = '

HTML 格式

' diff --git a/apps/ops/views.py b/apps/ops/views.py index ccff2aa21..b35fdb764 100644 --- a/apps/ops/views.py +++ b/apps/ops/views.py @@ -1,7 +1,8 @@ # -*- coding: utf-8 -*- # -from django.views.generic import TemplateView from django.conf import settings +from django.http import HttpResponse +from django.views.generic import TemplateView from common.views.mixins import PermissionsMixin from rbac.permissions import RBACPermission @@ -16,6 +17,11 @@ class CeleryTaskLogView(PermissionsMixin, TemplateView): 'GET': 'ops.view_celerytaskexecution' } + def get(self, request, *args, **kwargs): + if not request.user.is_authenticated: + return HttpResponse(status=401) + return super().get(request, *args, **kwargs) + def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) context.update({ From bcda879f3b76f8ab0ae67f2c9a20508b29cc3f74 Mon Sep 17 00:00:00 2001 From: ibuler Date: Wed, 13 Sep 2023 17:19:13 +0800 Subject: [PATCH 2/3] =?UTF-8?q?perf:=20=E4=BF=AE=E6=94=B9=20ticket=20?= =?UTF-8?q?=E8=AE=A4=E8=AF=81=E7=9A=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- apps/tickets/views/approve.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/tickets/views/approve.py b/apps/tickets/views/approve.py index 21e5b4af6..8dcda595f 100644 --- a/apps/tickets/views/approve.py +++ b/apps/tickets/views/approve.py @@ -4,6 +4,7 @@ from __future__ import unicode_literals from django.core.cache import cache +from django.http import HttpResponse from django.shortcuts import redirect, reverse from django.utils.translation import gettext as _ from django.views.generic.base import TemplateView @@ -79,6 +80,9 @@ class TicketDirectApproveView(TemplateView): return super().get_context_data(**kwargs) def get(self, request, *args, **kwargs): + if not request.user.is_authenticated: + return HttpResponse(status=401) + token = kwargs.get('token') ticket_info = cache.get(token) if not ticket_info: From b9997b07db173aa3ea52c1af5383d80acddb3f99 Mon Sep 17 00:00:00 2001 From: ibuler Date: Wed, 13 Sep 2023 17:22:50 +0800 Subject: [PATCH 3/3] =?UTF-8?q?perf:=20=E5=8E=BB=E6=8E=89=E4=B8=8D?= =?UTF-8?q?=E7=94=A8=E7=9A=84=20backend?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- apps/accounts/api/account/task.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/apps/accounts/api/account/task.py b/apps/accounts/api/account/task.py index 6bbeb94d7..16ea84eae 100644 --- a/apps/accounts/api/account/task.py +++ b/apps/accounts/api/account/task.py @@ -9,12 +9,9 @@ __all__ = [ 'AccountsTaskCreateAPI', ] -from rbac.permissions import RBACPermission - class AccountsTaskCreateAPI(CreateAPIView): serializer_class = serializers.AccountTaskSerializer - permission_classes = [RBACPermission] def check_permissions(self, request): act = request.data.get('action')