diff --git a/apps/common/exception/app_exception.py b/apps/common/exception/app_exception.py index 1de0a4b70a1..0e3ab882eea 100644 --- a/apps/common/exception/app_exception.py +++ b/apps/common/exception/app_exception.py @@ -1,11 +1,12 @@ # coding=utf-8 """ - @project: qabot - @Author：虎虎 - @file： app_exception.py - @date：2023/9/4 14:04 - @desc: +@project: qabot +@Author：虎虎 +@file： app_exception.py +@date：2023/9/4 14:04 +@desc: """ + from rest_framework import status @@ -13,6 +14,7 @@ class AppApiException(Exception): """ 项目内异常 """ + status_code = status.HTTP_200_OK def __init__(self, code, message): @@ -20,10 +22,23 @@ def __init__(self, code, message): self.message = message +class BadRequest400(AppApiException): + """ + 请求参数错误 + """ + + status_code = status.HTTP_400_BAD_REQUEST + + def __init__(self, code, message): + self.code = code + self.message = message + + class NotFound404(AppApiException): """ - 未认证(未登录)异常 - """ + 未认证(未登录)异常 + """ + status_code = status.HTTP_404_NOT_FOUND def __init__(self, code, message): @@ -35,6 +50,7 @@ class AppAuthenticationFailed(AppApiException): """ 未认证(未登录)异常 """ + status_code = status.HTTP_401_UNAUTHORIZED def __init__(self, code, message): @@ -46,6 +62,7 @@ class AppUnauthorizedFailed(AppApiException): """ 未授权(没有权限)异常 """ + status_code = status.HTTP_403_FORBIDDEN def __init__(self, code, message): @@ -57,6 +74,7 @@ class AppEmbedIdentityFailed(AppApiException): """ 嵌入cookie异常 """ + status_code = 460 def __init__(self, code, message): @@ -66,8 +84,9 @@ def __init__(self, code, message): class AppChatNumOutOfBoundsFailed(AppApiException): """ - 访问次数超过今日访问量 + 访问次数超过今日访问量 """ + status_code = 461 def __init__(self, code, message): diff --git a/apps/common/exception/handle_exception.py b/apps/common/exception/handle_exception.py index 50fce4aa763..a3d148a75a8 100644 --- a/apps/common/exception/handle_exception.py +++ b/apps/common/exception/handle_exception.py @@ -1,12 +1,12 @@ # coding=utf-8 """ - @project: qabot - @Author：虎虎 - @file： handle_exception.py - @date：2023/9/5 19:29 - @desc: +@project: qabot +@Author：虎虎 +@file： handle_exception.py +@date：2023/9/5 19:29 +@desc: """ -import logging + import traceback from rest_framework.exceptions import ValidationError, ErrorDetail, APIException @@ -29,19 +29,31 @@ def to_result(key, args, parent_key=None): :param parent_key 父key :return: 接口响应对象 """ - error_detail = list(filter( - lambda d: True if isinstance(d, ErrorDetail) else True if isinstance(d, dict) and len( - d.keys()) > 0 else False, - (args[0] if len(args) > 0 else {key: [ErrorDetail(_('Unknown exception'), code='unknown')]}).get(key)))[0] + error_detail = list( + filter( + lambda d: ( + True if isinstance(d, ErrorDetail) else True if isinstance(d, dict) and len(d.keys()) > 0 else False + ), + (args[0] if len(args) > 0 else {key: [ErrorDetail(_("Unknown exception"), code="unknown")]}).get(key), + ) + )[0] if isinstance(error_detail, dict): - return list(map(lambda k: to_result(k, args=[error_detail], - parent_key=key if parent_key is None else parent_key + '.' + key), - error_detail.keys() if len(error_detail) > 0 else []))[0] - - return result.Result(500 if isinstance(error_detail.code, str) else error_detail.code, - message=f"【{key if parent_key is None else parent_key + '.' + key}】为必填参数" if str( - error_detail) == "This field is required." else error_detail) + return list( + map( + lambda k: to_result( + k, args=[error_detail], parent_key=key if parent_key is None else parent_key + "." + key + ), + error_detail.keys() if len(error_detail) > 0 else [], + ) + )[0] + + return result.Result( + 500 if isinstance(error_detail.code, str) else error_detail.code, + message=f"【{key if parent_key is None else parent_key + '.' + key}】为必填参数" + if str(error_detail) == "This field is required." + else error_detail, + ) def validation_error_to_result(exc: ValidationError): @@ -52,11 +64,10 @@ def validation_error_to_result(exc: ValidationError): """ try: v = find_err_detail(exc.detail) - if v is None: - return result.error(str(exc.detail)) - return result.error(str(v)) - except Exception as e: - return result.error(str(exc.detail)) + message = str(v) if v is not None else str(exc.detail) + except Exception: + message = str(exc.detail) + return result.Result(code=400, message=message, response_status=400) def find_err_detail(exc_detail): @@ -77,7 +88,7 @@ def find_err_detail(exc_detail): if isinstance(_value, dict) and len(_value.keys()) > 0: try: return find_err_detail(ReturnDict(_value, serializer=exc_detail.serializer.fields[key])) - except Exception as e: + except Exception: return _value if isinstance(exc_detail, list): for v in exc_detail: @@ -89,7 +100,7 @@ def find_err_detail(exc_detail): def get_label(key, exc_detail): try: return exc_detail.serializer.fields[key].label - except Exception as e: + except Exception: return key @@ -105,6 +116,6 @@ def handle_exception(exc, context): if issubclass(exception_class, APIException): return result.error(exc.detail) if response is None: - maxkb_logger.error(f'{str(exc)}:{traceback.format_exc()}') - return result.error(str(exc)) + maxkb_logger.error(f"{str(exc)}:{traceback.format_exc()}") + return result.error(_("Unknown exception"), response_status=500) return response diff --git a/apps/common/handle/handle_exception.py b/apps/common/handle/handle_exception.py index 87afc4566be..0d3c73fb8e8 100644 --- a/apps/common/handle/handle_exception.py +++ b/apps/common/handle/handle_exception.py @@ -1,12 +1,12 @@ # coding=utf-8 """ - @project: qabot - @Author：虎 - @file： handle_exception.py - @date：2023/9/5 19:29 - @desc: +@project: qabot +@Author：虎 +@file： handle_exception.py +@date：2023/9/5 19:29 +@desc: """ -import logging + import traceback from rest_framework.exceptions import ValidationError, ErrorDetail, APIException @@ -28,19 +28,31 @@ def to_result(key, args, parent_key=None): :param parent_key 父key :return: 接口响应对象 """ - error_detail = list(filter( - lambda d: True if isinstance(d, ErrorDetail) else True if isinstance(d, dict) and len( - d.keys()) > 0 else False, - (args[0] if len(args) > 0 else {key: [ErrorDetail(_('Unknown exception'), code='unknown')]}).get(key)))[0] + error_detail = list( + filter( + lambda d: ( + True if isinstance(d, ErrorDetail) else True if isinstance(d, dict) and len(d.keys()) > 0 else False + ), + (args[0] if len(args) > 0 else {key: [ErrorDetail(_("Unknown exception"), code="unknown")]}).get(key), + ) + )[0] if isinstance(error_detail, dict): - return list(map(lambda k: to_result(k, args=[error_detail], - parent_key=key if parent_key is None else parent_key + '.' + key), - error_detail.keys() if len(error_detail) > 0 else []))[0] + return list( + map( + lambda k: to_result( + k, args=[error_detail], parent_key=key if parent_key is None else parent_key + "." + key + ), + error_detail.keys() if len(error_detail) > 0 else [], + ) + )[0] - return result.Result(500 if isinstance(error_detail.code, str) else error_detail.code, - message=f"【{key if parent_key is None else parent_key + '.' + key}】为必填参数" if str( - error_detail) == "This field is required." else error_detail) + return result.Result( + 500 if isinstance(error_detail.code, str) else error_detail.code, + message=f"【{key if parent_key is None else parent_key + '.' + key}】为必填参数" + if str(error_detail) == "This field is required." + else error_detail, + ) def validation_error_to_result(exc: ValidationError): @@ -51,11 +63,10 @@ def validation_error_to_result(exc: ValidationError): """ try: v = find_err_detail(exc.detail) - if v is None: - return result.error(str(exc.detail)) - return result.error(str(v)) - except Exception as e: - return result.error(str(exc.detail)) + message = str(v) if v is not None else str(exc.detail) + except Exception: + message = str(exc.detail) + return result.Result(code=400, message=message, response_status=400) def find_err_detail(exc_detail): @@ -90,6 +101,6 @@ def handle_exception(exc, context): if issubclass(exception_class, APIException): return result.error(exc.detail) if response is None: - maxkb_logger.error(f'{str(exc)}:{traceback.format_exc()}') - return result.error(str(exc)) + maxkb_logger.error(f"{str(exc)}:{traceback.format_exc()}") + return result.error(_("Unknown exception"), response_status=500) return response diff --git a/apps/common/tests/test_defect_exception_handler.py b/apps/common/tests/test_defect_exception_handler.py new file mode 100644 index 00000000000..02d7e3127fe --- /dev/null +++ b/apps/common/tests/test_defect_exception_handler.py @@ -0,0 +1,44 @@ +import json +from unittest.mock import patch + +from django.test import SimpleTestCase +from rest_framework.exceptions import ValidationError + +from common.exception import handle_exception as exception_handle_module +from common.exception.app_exception import BadRequest400 +from common.handle import handle_exception as common_handle_module + + +class ExceptionHandlerDefectTests(SimpleTestCase): + def test_bad_request_uses_http_400(self): + for module in (exception_handle_module, common_handle_module): + with self.subTest(module=module.__name__): + response = module.handle_exception(BadRequest400(400, "Invalid parameter"), {}) + + self.assertEqual(400, response.status_code) + self.assertEqual(400, json.loads(response.content)["code"]) + + def test_validation_error_uses_http_400(self): + for module in (exception_handle_module, common_handle_module): + with self.subTest(module=module.__name__): + response = module.handle_exception(ValidationError("Invalid parameter"), {}) + + self.assertEqual(400, response.status_code) + self.assertEqual(400, json.loads(response.content)["code"]) + + def test_unhandled_exception_hides_internal_details(self): + internal_message = "secret database path /opt/maxkb/private" + + for module in (exception_handle_module, common_handle_module): + with ( + self.subTest(module=module.__name__), + patch(f"{module.__name__}.maxkb_logger") as logger, + ): + response = module.handle_exception(RuntimeError(internal_message), {}) + payload = json.loads(response.content) + + self.assertEqual(500, response.status_code) + self.assertEqual(500, payload["code"]) + self.assertNotIn("secret", str(payload["message"]).lower()) + self.assertNotIn("/opt/maxkb", str(payload["message"])) + logger.error.assert_called_once() diff --git a/apps/models_provider/impl/aliyun_bai_lian_model_provider/aliyun_bai_lian_model_provider.py b/apps/models_provider/impl/aliyun_bai_lian_model_provider/aliyun_bai_lian_model_provider.py index fb554ff1712..4712987797b 100644 --- a/apps/models_provider/impl/aliyun_bai_lian_model_provider/aliyun_bai_lian_model_provider.py +++ b/apps/models_provider/impl/aliyun_bai_lian_model_provider/aliyun_bai_lian_model_provider.py @@ -237,43 +237,43 @@ def _type_impl_map(): ModelTypeConst.LLM: ( aliyun_bai_lian_llm_model_credential, BaiLianChatModel, - _("Alibaba Cloud Bailian LLM model"), + _(""), ), ModelTypeConst.EMBEDDING: ( aliyun_bai_lian_embedding_model_credential, AliyunBaiLianEmbedding, - _("Alibaba Cloud Bailian embedding model"), + _(""), ), ModelTypeConst.STT: ( aliyun_bai_lian_stt_model_credential, AliyunBaiLianSpeechToText, - _("Alibaba Cloud Bailian speech-to-text model"), + _(""), ), ModelTypeConst.TTS: ( aliyun_bai_lian_tts_model_credential, AliyunBaiLianTextToSpeech, - _("Alibaba Cloud Bailian TTS model"), + _(""), ), - ModelTypeConst.IMAGE: (qwenvl_model_credential, QwenVLChatModel, _("Alibaba Cloud Bailian vision model")), + ModelTypeConst.IMAGE: (qwenvl_model_credential, QwenVLChatModel, _("")), ModelTypeConst.TTI: ( qwentti_model_credential, QwenTextToImageModel, - _("Alibaba Cloud Bailian image generation model"), + _(""), ), ModelTypeConst.RERANKER: ( aliyun_bai_lian_model_credential, AliyunBaiLianReranker, - _("Alibaba Cloud Bailian rerank model"), + _(""), ), ModelTypeConst.TTV: ( aliyun_bai_lian_ttv_model_credential, GenerationVideoModel, - _("Alibaba Cloud Bailian text-to-video model"), + _(""), ), ModelTypeConst.ITV: ( aliyun_bai_lian_itv_model_credential, GenerationVideoModel, - _("Alibaba Cloud Bailian image-to-video model"), + _(""), ), } diff --git a/apps/models_provider/impl/aliyun_bai_lian_model_provider/model/image.py b/apps/models_provider/impl/aliyun_bai_lian_model_provider/model/image.py index b6ed53f8f6d..b55b980806f 100644 --- a/apps/models_provider/impl/aliyun_bai_lian_model_provider/model/image.py +++ b/apps/models_provider/impl/aliyun_bai_lian_model_provider/model/image.py @@ -1,7 +1,9 @@ # coding=utf-8 from typing import Dict +from langchain_core.messages import HumanMessage +from common.exception.app_exception import AppApiException from models_provider.base_model_provider import MaxKBBaseModel from models_provider.impl.base_chat_open_ai import BaseChatOpenAI @@ -13,6 +15,9 @@ def is_cache_model(): @staticmethod def new_instance(model_type, model_name, model_credential: Dict[str, object], **model_kwargs): + if str(model_name).lower().startswith("qwen-image"): + raise AppApiException(500, "qwen-image 是图像生成模型，不能作为视觉理解模型使用") + QwenVLChatModel._validate_api_key(model_credential.get("api_key")) optional_params = MaxKBBaseModel.filter_optional_params(model_kwargs) chat_tong_yi = QwenVLChatModel( model_name=model_name, @@ -25,5 +30,14 @@ def new_instance(model_type, model_name, model_credential: Dict[str, object], ** ) return chat_tong_yi + @staticmethod + def _validate_api_key(api_key): + try: + str(api_key).encode("ascii") + except UnicodeEncodeError as error: + raise AppApiException(500, "API Key 只能是 ASCII 字符，请检查是否包含中文标点或空格") from error + def check_auth(self, api_key): + self._validate_api_key(api_key) + self.invoke([HumanMessage(content=[{"type": "text", "text": "1"}])]) return True diff --git a/apps/models_provider/serializers/model_serializer.py b/apps/models_provider/serializers/model_serializer.py index 5f91985ddf8..133083f5c99 100644 --- a/apps/models_provider/serializers/model_serializer.py +++ b/apps/models_provider/serializers/model_serializer.py @@ -10,7 +10,7 @@ from common.config.embedding_config import ModelManage from common.database_model_manage.database_model_manage import DatabaseModelManage from common.db.search import native_search -from common.exception.app_exception import AppApiException +from common.exception.app_exception import AppApiException, BadRequest400, NotFound404 from common.utils.common import get_file_content from common.utils.rsa_util import rsa_long_decrypt, rsa_long_encrypt from django.db import transaction @@ -138,27 +138,38 @@ def is_valid(self, *, raise_exception=False): model_query = model_query.filter(workspace_id=workspace_id) model = model_query.first() if model is None: - raise AppApiException(500, _("Model does not exist")) + raise NotFound404(404, _("Model does not exist")) if model.workspace_id == "None": raise AppApiException(500, _("Shared models cannot be deleted or modified")) def one(self, with_valid=False): if with_valid: - super().is_valid(raise_exception=True) - model = QuerySet(Model).get(id=self.data.get("id"), workspace_id=self.data.get("workspace_id", "None")) + self.is_valid(raise_exception=True) + model = ( + QuerySet(Model) + .filter( + id=self.initial_data.get("id"), + workspace_id=self.initial_data.get("workspace_id", "None"), + ) + .first() + ) + if model is None: + raise NotFound404(404, _("Model does not exist")) return ModelSerializer.model_to_dict(model) def one_meta(self, with_valid=False): - model = None if with_valid: - super().is_valid(raise_exception=True) - model = ( - QuerySet(Model) - .filter(id=self.data.get("id"), workspace_id=self.data.get("workspace_id", "None")) - .first() + self.is_valid(raise_exception=True) + model = ( + QuerySet(Model) + .filter( + id=self.initial_data.get("id"), + workspace_id=self.initial_data.get("workspace_id", "None"), ) + .first() + ) if model is None: - raise AppApiException(500, _("Model does not exist")) + raise NotFound404(404, _("Model does not exist")) return { "id": str(model.id), "provider": model.provider, @@ -210,7 +221,7 @@ def delete(self, with_valid=True): def edit(self, instance: Dict, user_id: str, with_valid=True): if with_valid: - super().is_valid(raise_exception=True) + self.is_valid(raise_exception=True) model = ( QuerySet(Model) .filter(id=self.data.get("id"), workspace_id=self.data.get("workspace_id", "None")) @@ -219,19 +230,20 @@ def edit(self, instance: Dict, user_id: str, with_valid=True): credential, model_credential, provider_handler = ModelSerializer.Edit(data={**instance}).is_valid( model=model ) - try: - model.status = Status.SUCCESS - default_params = {item["field"]: item["default_value"] for item in model.model_params_form} - # 校验模型认证数据 - provider_handler.is_valid_credential( - model.model_type, instance.get("model_name"), credential, default_params, raise_exception=True - ) + if any(instance.get(key) is not None for key in ("credential", "model_type", "model_name")): + try: + model.status = Status.SUCCESS + default_params = {item["field"]: item["default_value"] for item in model.model_params_form} + # 仅连接凭据或基础模型变化时校验供应商连接，纯名称修改不应依赖外部服务可用性。 + provider_handler.is_valid_credential( + model.model_type, instance.get("model_name"), credential, default_params, raise_exception=True + ) - except AppApiException as e: - if e.code == ValidCode.model_not_fount: - model.status = Status.DOWNLOAD - else: - raise e + except AppApiException as e: + if e.code == ValidCode.model_not_fount: + model.status = Status.DOWNLOAD + else: + raise e update_keys = ["credential", "name", "model_type", "model_name"] for update_key in update_keys: if update_key in instance and instance.get(update_key) is not None: @@ -273,13 +285,15 @@ def is_valid(self, model=None, raise_exception=False): ModelSerializer.model_to_dict(model) provider = model.provider - model_type = self.data.get("model_type") - model_name = self.data.get("model_name") + model_type = self.data.get("model_type") or model.model_type + model_name = self.data.get("model_name") or model.model_name credential = self.data.get("credential") provider_handler = ModelProvideConstants[provider] model_credential = ModelProvideConstants[provider].get_model_credential(model_type, model_name) source_model_credential = json.loads(rsa_long_decrypt(model.credential)) source_encryption_model_credential = model_credential.encryption_dict(source_model_credential) + if credential is None: + credential = source_model_credential if credential is not None: for k in source_encryption_model_credential.keys(): if k in credential and credential[k] == source_encryption_model_credential[k]: @@ -297,7 +311,27 @@ class Create(serializers.Serializer): workspace_id = serializers.CharField(required=False, label=_("workspace id"), max_length=128) def is_valid(self, *, raise_exception=False): - super().is_valid(raise_exception=True) + if not super().is_valid(raise_exception=raise_exception): + return False + provider = self.data.get("provider") + try: + provider_handler = ModelProvideConstants[provider] + except KeyError as error: + raise BadRequest400(400, _("provider is invalid")) from error + model_params_form = self.data.get("model_params_form") + if not isinstance(model_params_form, list): + raise BadRequest400(400, _("model_params_form must be a list")) + for index, item in enumerate(model_params_form): + if not isinstance(item, dict): + raise BadRequest400( + 400, + _("The {index}th item in model_params_form must be a dictionary").format(index=index), + ) + if "field" not in item or "default_value" not in item: + raise BadRequest400( + 400, + _("The field and default_value fields are required for the {index}th item").format(index=index), + ) if ( QuerySet(Model) .filter(name=self.data.get("name"), workspace_id=self.data.get("workspace_id", "None")) @@ -306,14 +340,15 @@ def is_valid(self, *, raise_exception=False): raise AppApiException( 500, _("base model【{model_name}】already exists").format(model_name=self.data.get("name")) ) - default_params = {item["field"]: item["default_value"] for item in self.data.get("model_params_form")} - ModelProvideConstants[self.data.get("provider")].is_valid_credential( + default_params = {item["field"]: item["default_value"] for item in model_params_form} + provider_handler.is_valid_credential( self.data.get("model_type"), self.data.get("model_name"), self.data.get("credential"), default_params, raise_exception=True, ) + return True def insert(self, workspace_id, with_valid=True): status = Status.SUCCESS @@ -511,13 +546,10 @@ def is_valid(self, *, raise_exception=False): ) if model is None: - raise AppApiException(500, _("Model does not exist")) - - if model.workspace_id == "None": - return model + raise NotFound404(404, _("Model does not exist")) - if model.workspace_id != validated_data["workspace_id"]: - raise AppApiException(500, _("Model does not exist")) + if model.workspace_id != validated_data.get("workspace_id"): + raise NotFound404(404, _("Model does not exist")) return model diff --git a/apps/models_provider/tests/test_provide_validation.py b/apps/models_provider/tests/test_provide_validation.py new file mode 100644 index 00000000000..93767153654 --- /dev/null +++ b/apps/models_provider/tests/test_provide_validation.py @@ -0,0 +1,99 @@ +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from django.http import QueryDict +from django.test import SimpleTestCase + +from common.exception.app_exception import BadRequest400 +from models_provider.views.provide import Provide + + +def request_with_params(**params): + query_params = QueryDict(mutable=True) + query_params.update(params) + return SimpleNamespace(query_params=query_params) + + +class ProvideParameterValidationTests(SimpleTestCase): + def test_model_type_list_requires_provider(self): + with self.assertRaises(BadRequest400): + Provide.ModelTypeList().get(request_with_params()) + + def test_model_params_form_requires_all_query_parameters(self): + request = request_with_params(provider="model_openai_provider", model_type="LLM") + + with self.assertRaises(BadRequest400): + Provide.ModelParamsForm().get(request) + + def test_model_form_requires_all_query_parameters(self): + request = request_with_params(provider="model_openai_provider", model_name="gpt-4o") + + with self.assertRaises(BadRequest400): + Provide.ModelForm().get(request) + + def test_model_form_allows_empty_model_name(self): + provider = MagicMock() + provider.get_model_credential.return_value.to_form_list.return_value = [{"field": "api_key"}] + request = request_with_params( + provider="test_provider", + model_type="LLM", + model_name="", + ) + + with patch("models_provider.views.provide.ModelProvideConstants", {"test_provider": provider}): + response = Provide.ModelForm().get(request) + + self.assertEqual(200, response.status_code) + provider.get_model_credential.assert_called_once_with("LLM", "") + + def test_unknown_provider_is_bad_request(self): + requests = ( + (Provide.ModelTypeList(), request_with_params(provider="missing")), + (Provide.ModelList(), request_with_params(provider="missing", model_type="LLM")), + ( + Provide.ModelParamsForm(), + request_with_params(provider="missing", model_type="LLM", model_name="missing"), + ), + ( + Provide.ModelForm(), + request_with_params(provider="missing", model_type="LLM", model_name="missing"), + ), + ) + + for view, request in requests: + with self.subTest(view=view.__class__.__name__), self.assertRaises(BadRequest400): + view.get(request) + + def test_model_list_requires_provider_and_model_type(self): + requests = ( + request_with_params(model_type="LLM"), + request_with_params(provider="model_openai_provider"), + ) + + for request in requests: + with self.subTest(params=request), self.assertRaises(BadRequest400): + Provide.ModelList().get(request) + + def test_model_list_rejects_invalid_model_type(self): + provider = MagicMock() + provider.get_model_type_list.return_value = [{"value": "LLM"}] + + with patch("models_provider.views.provide.ModelProvideConstants", {"test_provider": provider}): + with self.assertRaises(BadRequest400): + Provide.ModelList().get(request_with_params(provider="test_provider", model_type="INVALID")) + + def test_model_list_returns_provider_models(self): + provider = MagicMock() + provider.get_model_type_list.return_value = [{"value": "LLM"}] + provider.get_model_list.return_value = [{"name": "test-model"}] + + with patch("models_provider.views.provide.ModelProvideConstants", {"test_provider": provider}): + response = Provide.ModelList().get(request_with_params(provider="test_provider", model_type="LLM")) + + self.assertEqual(200, response.status_code) + provider.get_model_list.assert_called_once_with( + "LLM", + api_key=None, + api_base=None, + credential=None, + ) diff --git a/apps/models_provider/views/provide.py b/apps/models_provider/views/provide.py index d6ec2f0dbf1..69de1b4016e 100644 --- a/apps/models_provider/views/provide.py +++ b/apps/models_provider/views/provide.py @@ -12,6 +12,22 @@ from rest_framework.request import Request from rest_framework.views import APIView +from common.exception.app_exception import BadRequest400 + + +def _require_query_params(query_params, names): + for name in names: + if not query_params.get(name): + raise BadRequest400(400, _("%(key)s is required") % {"key": name}) + + +def _get_provider(provider): + if not provider: + raise BadRequest400(400, _("provider is required")) + if provider not in ModelProvideConstants: + raise BadRequest400(400, _("provider is invalid")) + return ModelProvideConstants[provider] + class Provide(APIView): authentication_classes = [TokenAuth] @@ -59,7 +75,7 @@ class ModelTypeList(APIView): ) def get(self, request: Request): provider = request.query_params.get("provider") - return result.success(ModelProvideConstants[provider].get_model_type_list()) + return result.success(_get_provider(provider).get_model_type_list()) class ModelList(APIView): authentication_classes = [TokenAuth] @@ -75,7 +91,13 @@ class ModelList(APIView): ) @staticmethod def _get_model_list(provider, model_type, api_key, api_base, credential, search): - model_list = ModelProvideConstants[provider].get_model_list( + provider_handler = _get_provider(provider) + if not model_type: + raise BadRequest400(400, _("model_type is required")) + model_type_list = [item.get("value") for item in provider_handler.get_model_type_list()] + if model_type not in model_type_list: + raise BadRequest400(400, _("model_type is invalid")) + model_list = provider_handler.get_model_list( model_type, api_key=api_key, api_base=api_base, @@ -163,9 +185,11 @@ class ModelParamsForm(APIView): tags=[_("Model")], # type: ignore ) def get(self, request: Request): + _require_query_params(request.query_params, ("provider", "model_type", "model_name")) provider = request.query_params.get("provider") model_type = request.query_params.get("model_type") model_name = request.query_params.get("model_name") + _get_provider(provider) return result.success(get_default_model_params_setting(provider, model_type, model_name)) @@ -182,9 +206,8 @@ class ModelForm(APIView): tags=[_("Model")], # type: ignore ) def get(self, request: Request): + _require_query_params(request.query_params, ("provider", "model_type")) provider = request.query_params.get("provider") model_type = request.query_params.get("model_type") model_name = request.query_params.get("model_name") - return result.success( - ModelProvideConstants[provider].get_model_credential(model_type, model_name).to_form_list() - ) + return result.success(_get_provider(provider).get_model_credential(model_type, model_name).to_form_list()) diff --git a/apps/system_manage/serializers/chat_user.py b/apps/system_manage/serializers/chat_user.py index cce94b86b10..561e73e1bd4 100644 --- a/apps/system_manage/serializers/chat_user.py +++ b/apps/system_manage/serializers/chat_user.py @@ -393,7 +393,8 @@ def edit(self, instance, with_valid=True): user = ChatUser.objects.filter(id=self.data.get("id")).first() self._update_user_fields(user, instance) user.save() - add_or_edit_user_group_relation(user, instance.get("user_group_ids", [])) + if "user_group_ids" in instance: + add_or_edit_user_group_relation(user, instance.get("user_group_ids", [])) return ChatUserInstanceSerializer(user).data @staticmethod diff --git a/apps/system_manage/tests.py b/apps/system_manage/tests.py index 7ce503c2dd9..f5e15c15c1b 100644 --- a/apps/system_manage/tests.py +++ b/apps/system_manage/tests.py @@ -1,3 +1,80 @@ -from django.test import TestCase +from types import SimpleNamespace +from unittest.mock import patch -# Create your tests here. +from django.test import SimpleTestCase, TestCase + +from common.auth.constants.permission_constants import PermissionConstants +from common.auth.struct.aggregate_permission import AggregatePermission +from common.exception.app_exception import AppUnauthorizedFailed +from system_manage.models import ChatUser, UserGroup, UserGroupRelation +from system_manage.serializers.chat_user import ChatUserSerializer +from system_manage.views import resource_chat_user_group +from system_manage.views.resource_chat_user import ResourceChatUser, get_chat_user_permission_constant + + +class ChatUserPermissionTests(SimpleTestCase): + @staticmethod + def build_permissions(items, request, kwargs, compare): + for item in items: + if callable(item) and not isinstance(item, AggregatePermission): + item(request, kwargs) + return SimpleNamespace(hasPermission=lambda *args, **kwargs: True) + + def test_supported_resources_keep_existing_permissions(self): + self.assertEqual( + PermissionConstants.APPLICATION_CHAT_USER_READ, + get_chat_user_permission_constant("APPLICATION", "READ"), + ) + self.assertEqual( + PermissionConstants.KNOWLEDGE_CHAT_USER_EDIT, + get_chat_user_permission_constant("KNOWLEDGE", "EDIT"), + ) + + def test_tool_resource_returns_forbidden_without_i18n_key(self): + with self.assertRaises(AppUnauthorizedFailed) as context: + get_chat_user_permission_constant("TOOL", "READ") + + self.assertEqual(403, context.exception.status_code) + self.assertNotIn("CHAT_USER", str(context.exception.message)) + + def test_list_page_and_group_endpoints_reject_tool_permission(self): + kwargs = { + "workspace_id": "workspace-1", + "resource_type": "TOOL", + "resource_id": "resource-1", + "user_group_id": "group-1", + } + + with patch("common.auth.authentication._build", side_effect=self.build_permissions): + with self.assertRaises(AppUnauthorizedFailed): + ResourceChatUser.get(ResourceChatUser(), SimpleNamespace(), **kwargs) + with self.assertRaises(AppUnauthorizedFailed): + ResourceChatUser.Page.get( + ResourceChatUser.Page(), + SimpleNamespace(), + **kwargs, + current_page=1, + page_size=20, + ) + with self.assertRaises(AppUnauthorizedFailed): + resource_chat_user_group.ResourceChatUserGroup.get( + resource_chat_user_group.ResourceChatUserGroup(), SimpleNamespace(), **kwargs + ) + + +class ChatUserEditTests(TestCase): + def test_status_and_password_changes_preserve_user_groups(self): + user = ChatUser.objects.create( + username="chat-user", + nick_name="Chat User", + password="encrypted-password", + ) + group = UserGroup.objects.create(name="chat-user-group") + UserGroupRelation.objects.create(user=user, group=group) + operate = ChatUserSerializer.Operate(data={"id": user.id}) + + operate.edit({"is_active": False}, with_valid=True) + operate.edit({"is_active": True}, with_valid=True) + operate.re_password({"password": "Password1!", "re_password": "Password1!"}, with_valid=True) + + self.assertTrue(UserGroupRelation.objects.filter(user=user, group=group).exists()) diff --git a/apps/system_manage/views/resource_chat_user.py b/apps/system_manage/views/resource_chat_user.py index 70e9026a108..afb1f3b4c13 100644 --- a/apps/system_manage/views/resource_chat_user.py +++ b/apps/system_manage/views/resource_chat_user.py @@ -20,6 +20,7 @@ from common.auth.constants.role_constants import RoleConstants from common.auth.struct.aggregate_permission import ViewPermission from common.auth.struct.permission import Permission +from common.exception.app_exception import AppUnauthorizedFailed from system_manage.api.resource_chat_user import ( WorkspaceResourceChatUserListAPI, @@ -29,6 +30,16 @@ from system_manage.serializers.resource_chat_user import ResourceChatUserQuerySerializer, ResourceChatUserSerializer +_CHAT_USER_RESOURCE_TYPES = {"APPLICATION", "KNOWLEDGE"} + + +def get_chat_user_permission_constant(resource_type, operate): + normalized = str(resource_type or "").upper() + if normalized not in _CHAT_USER_RESOURCE_TYPES: + raise AppUnauthorizedFailed(403, _("No permission to access")) + return PermissionConstants[f"{normalized}_CHAT_USER_{operate}"] + + class ResourceChatUser(APIView): authentication_classes = [TokenAuth] @@ -42,20 +53,20 @@ class ResourceChatUser(APIView): tags=[_("Chat user")], # type: ignore ) @has_permissions( - lambda r, kwargs: PermissionConstants[ - f"{kwargs.get('resource_type').upper()}_CHAT_USER_READ" - ].get_workspace_permission(), - lambda r, kwargs: PermissionConstants[ - f"{kwargs.get('resource_type').upper()}_CHAT_USER_READ" - ].get_workspace_permission_workspace_manage_role(), + lambda r, kwargs: get_chat_user_permission_constant( + kwargs.get("resource_type"), "READ" + ).get_workspace_permission(), + lambda r, kwargs: get_chat_user_permission_constant( + kwargs.get("resource_type"), "READ" + ).get_workspace_permission_workspace_manage_role(), ViewPermission( [RoleConstants.USER.get_workspace_role()], [ lambda r, kwargs: Permission( - group=PermissionConstants[kwargs.get("resource_type").upper()].value.group, - sub_group=PermissionConstants[kwargs.get("resource_type").upper()].value.sub_group, - operate=PermissionConstants[kwargs.get("resource_type").upper()].value.operate, - bit_index=PermissionConstants[kwargs.get("resource_type").upper()].value.bit_index, + group=get_chat_user_permission_constant(kwargs.get("resource_type"), "READ").value.group, + sub_group=get_chat_user_permission_constant(kwargs.get("resource_type"), "READ").value.sub_group, + operate=get_chat_user_permission_constant(kwargs.get("resource_type"), "READ").value.operate, + bit_index=get_chat_user_permission_constant(kwargs.get("resource_type"), "READ").value.bit_index, workspace_id=kwargs.get("workspace_id"), resource_id=kwargs.get("resource_id"), ) @@ -90,20 +101,20 @@ def get(self, request: Request, workspace_id: str, resource_type: str, resource_ tags=[_("Chat user")], # type: ignore ) @has_permissions( - lambda r, kwargs: PermissionConstants[ - f"{kwargs.get('resource_type').upper()}_CHAT_USER_EDIT" - ].get_workspace_permission(), - lambda r, kwargs: PermissionConstants[ - f"{kwargs.get('resource_type').upper()}_CHAT_USER_EDIT" - ].get_workspace_permission_workspace_manage_role(), + lambda r, kwargs: get_chat_user_permission_constant( + kwargs.get("resource_type"), "EDIT" + ).get_workspace_permission(), + lambda r, kwargs: get_chat_user_permission_constant( + kwargs.get("resource_type"), "EDIT" + ).get_workspace_permission_workspace_manage_role(), ViewPermission( [RoleConstants.USER.get_workspace_role()], [ lambda r, kwargs: Permission( - group=PermissionConstants[kwargs.get("resource_type").upper()].value.group, - sub_group=PermissionConstants[kwargs.get("resource_type").upper()].value.sub_group, - operate=PermissionConstants[kwargs.get("resource_type").upper()].value.operate, - bit_index=PermissionConstants[kwargs.get("resource_type").upper()].value.bit_index, + group=get_chat_user_permission_constant(kwargs.get("resource_type"), "EDIT").value.group, + sub_group=get_chat_user_permission_constant(kwargs.get("resource_type"), "EDIT").value.sub_group, + operate=get_chat_user_permission_constant(kwargs.get("resource_type"), "EDIT").value.operate, + bit_index=get_chat_user_permission_constant(kwargs.get("resource_type"), "EDIT").value.bit_index, workspace_id=kwargs.get("workspace_id"), resource_id=kwargs.get("resource_id"), ) @@ -137,20 +148,24 @@ class Page(APIView): tags=[_("Chat user")], # type: ignore ) @has_permissions( - lambda r, kwargs: PermissionConstants[ - f"{kwargs.get('resource_type').upper()}_CHAT_USER_READ" - ].get_workspace_permission(), - lambda r, kwargs: PermissionConstants[ - f"{kwargs.get('resource_type').upper()}_CHAT_USER_READ" - ].get_workspace_permission_workspace_manage_role(), + lambda r, kwargs: get_chat_user_permission_constant( + kwargs.get("resource_type"), "READ" + ).get_workspace_permission(), + lambda r, kwargs: get_chat_user_permission_constant( + kwargs.get("resource_type"), "READ" + ).get_workspace_permission_workspace_manage_role(), ViewPermission( [RoleConstants.USER.get_workspace_role()], [ lambda r, kwargs: Permission( - group=PermissionConstants[kwargs.get("resource_type").upper()].value.group, - sub_group=PermissionConstants[kwargs.get("resource_type").upper()].value.sub_group, - operate=PermissionConstants[kwargs.get("resource_type").upper()].value.operate, - bit_index=PermissionConstants[kwargs.get("resource_type").upper()].value.bit_index, + group=get_chat_user_permission_constant(kwargs.get("resource_type"), "READ").value.group, + sub_group=get_chat_user_permission_constant( + kwargs.get("resource_type"), "READ" + ).value.sub_group, + operate=get_chat_user_permission_constant(kwargs.get("resource_type"), "READ").value.operate, + bit_index=get_chat_user_permission_constant( + kwargs.get("resource_type"), "READ" + ).value.bit_index, workspace_id=kwargs.get("workspace_id"), resource_id=kwargs.get("resource_id"), ) diff --git a/apps/system_manage/views/resource_chat_user_group.py b/apps/system_manage/views/resource_chat_user_group.py index 55c9d139731..fa56710cd21 100644 --- a/apps/system_manage/views/resource_chat_user_group.py +++ b/apps/system_manage/views/resource_chat_user_group.py @@ -6,7 +6,6 @@ from common.auth import TokenAuth from common.auth.authentication import has_permissions from common.auth.constants.compare_constants import CompareConstants -from common.auth.constants.permission_constants import PermissionConstants from common.auth.constants.role_constants import RoleConstants from common.auth.struct.aggregate_permission import ViewPermission from common.auth.struct.permission import Permission @@ -17,6 +16,7 @@ ResourceChatUserGroupQuerySerializer, ResourceChatUserGroupSerializer, ) +from system_manage.views.resource_chat_user import get_chat_user_permission_constant class ResourceChatUserGroup(APIView): @@ -32,20 +32,20 @@ class ResourceChatUserGroup(APIView): tags=[_("Chat user")], # type: ignore ) @has_permissions( - lambda r, kwargs: PermissionConstants[ - f"{kwargs.get('resource_type').upper()}_CHAT_USER_READ" - ].get_workspace_permission(), - lambda r, kwargs: PermissionConstants[ - f"{kwargs.get('resource_type').upper()}_CHAT_USER_READ" - ].get_workspace_permission_workspace_manage_role(), + lambda r, kwargs: get_chat_user_permission_constant( + kwargs.get("resource_type"), "READ" + ).get_workspace_permission(), + lambda r, kwargs: get_chat_user_permission_constant( + kwargs.get("resource_type"), "READ" + ).get_workspace_permission_workspace_manage_role(), ViewPermission( [RoleConstants.USER.get_workspace_role()], [ lambda r, kwargs: Permission( - group=PermissionConstants[kwargs.get("resource_type").upper()].value.group, - sub_group=PermissionConstants[kwargs.get("resource_type").upper()].value.sub_group, - operate=PermissionConstants[kwargs.get("resource_type").upper()].value.operate, - bit_index=PermissionConstants[kwargs.get("resource_type").upper()].value.bit_index, + group=get_chat_user_permission_constant(kwargs.get("resource_type"), "READ").value.group, + sub_group=get_chat_user_permission_constant(kwargs.get("resource_type"), "READ").value.sub_group, + operate=get_chat_user_permission_constant(kwargs.get("resource_type"), "READ").value.operate, + bit_index=get_chat_user_permission_constant(kwargs.get("resource_type"), "READ").value.bit_index, workspace_id=kwargs.get("workspace_id"), resource_id=kwargs.get("resource_id"), ) @@ -83,20 +83,20 @@ def get( tags=[_("Chat user")], # type: ignore ) @has_permissions( - lambda r, kwargs: PermissionConstants[ - f"{kwargs.get('resource_type').upper()}_CHAT_USER_EDIT" - ].get_workspace_permission(), - lambda r, kwargs: PermissionConstants[ - f"{kwargs.get('resource_type').upper()}_CHAT_USER_EDIT" - ].get_workspace_permission_workspace_manage_role(), + lambda r, kwargs: get_chat_user_permission_constant( + kwargs.get("resource_type"), "EDIT" + ).get_workspace_permission(), + lambda r, kwargs: get_chat_user_permission_constant( + kwargs.get("resource_type"), "EDIT" + ).get_workspace_permission_workspace_manage_role(), ViewPermission( [RoleConstants.USER.get_workspace_role()], [ lambda r, kwargs: Permission( - group=PermissionConstants[kwargs.get("resource_type").upper()].value.group, - sub_group=PermissionConstants[kwargs.get("resource_type").upper()].value.sub_group, - operate=PermissionConstants[kwargs.get("resource_type").upper()].value.operate, - bit_index=PermissionConstants[kwargs.get("resource_type").upper()].value.bit_index, + group=get_chat_user_permission_constant(kwargs.get("resource_type"), "EDIT").value.group, + sub_group=get_chat_user_permission_constant(kwargs.get("resource_type"), "EDIT").value.sub_group, + operate=get_chat_user_permission_constant(kwargs.get("resource_type"), "EDIT").value.operate, + bit_index=get_chat_user_permission_constant(kwargs.get("resource_type"), "EDIT").value.bit_index, workspace_id=kwargs.get("workspace_id"), resource_id=kwargs.get("resource_id"), ) @@ -129,20 +129,24 @@ class Page(APIView): tags=[_("Chat user")], # type: ignore ) @has_permissions( - lambda r, kwargs: PermissionConstants[ - f"{kwargs.get('resource_type').upper()}_CHAT_USER_READ" - ].get_workspace_permission(), - lambda r, kwargs: PermissionConstants[ - f"{kwargs.get('resource_type').upper()}_CHAT_USER_READ" - ].get_workspace_permission_workspace_manage_role(), + lambda r, kwargs: get_chat_user_permission_constant( + kwargs.get("resource_type"), "READ" + ).get_workspace_permission(), + lambda r, kwargs: get_chat_user_permission_constant( + kwargs.get("resource_type"), "READ" + ).get_workspace_permission_workspace_manage_role(), ViewPermission( [RoleConstants.USER.get_workspace_role()], [ lambda r, kwargs: Permission( - group=PermissionConstants[kwargs.get("resource_type").upper()].value.group, - sub_group=PermissionConstants[kwargs.get("resource_type").upper()].value.sub_group, - operate=PermissionConstants[kwargs.get("resource_type").upper()].value.operate, - bit_index=PermissionConstants[kwargs.get("resource_type").upper()].value.bit_index, + group=get_chat_user_permission_constant(kwargs.get("resource_type"), "READ").value.group, + sub_group=get_chat_user_permission_constant( + kwargs.get("resource_type"), "READ" + ).value.sub_group, + operate=get_chat_user_permission_constant(kwargs.get("resource_type"), "READ").value.operate, + bit_index=get_chat_user_permission_constant( + kwargs.get("resource_type"), "READ" + ).value.bit_index, workspace_id=kwargs.get("workspace_id"), resource_id=kwargs.get("resource_id"), ) diff --git a/apps/trigger/serializers/trigger_task.py b/apps/trigger/serializers/trigger_task.py index baa28e0a162..3ccf7ed19f7 100644 --- a/apps/trigger/serializers/trigger_task.py +++ b/apps/trigger/serializers/trigger_task.py @@ -1,11 +1,12 @@ # coding=utf-8 """ - @project: MaxKB - @Author：虎虎 - @file： trigger_task.py - @date：2026/1/14 16:34 - @desc: +@project: MaxKB +@Author：虎虎 +@file： trigger_task.py +@date：2026/1/14 16:34 +@desc: """ + import os from django.db import models @@ -26,10 +27,25 @@ class ChatRecordSerializerModel(serializers.ModelSerializer): class Meta: model = ChatRecord - fields = ['id', 'chat_id', 'vote_status', 'vote_reason', 'vote_other_content', 'problem_text', 'answer_text', - 'message_tokens', 'answer_tokens', 'const', 'improve_paragraph_id_list', 'run_time', 'index', - 'answer_text_list', 'details', - 'create_time', 'update_time'] + fields = [ + "id", + "chat_id", + "vote_status", + "vote_reason", + "vote_other_content", + "problem_text", + "answer_text", + "message_tokens", + "answer_tokens", + "const", + "improve_paragraph_id_list", + "run_time", + "index", + "answer_text_list", + "details", + "create_time", + "update_time", + ] class TriggerTaskResponse(serializers.ModelSerializer): @@ -44,16 +60,17 @@ class TriggerTaskQuerySerializer(serializers.Serializer): def is_valid(self, *, raise_exception=False): super().is_valid(raise_exception=True) - workspace_id = self.data.get('workspace_id') - query_set = QuerySet(Trigger).filter(id=self.data.get('trigger_id')) + workspace_id = self.data.get("workspace_id") + query_set = QuerySet(Trigger).filter(id=self.data.get("trigger_id")) if workspace_id: query_set = query_set.filter(workspace_id=workspace_id) if not query_set.exists(): - raise AppApiException(500, _('Trigger id does not exist')) + raise AppApiException(500, _("Trigger id does not exist")) def get_query_set(self): - query_set = QuerySet(TriggerTask).filter(workspace_id=self.data.get("workspace_id")).filter( - trigger_id=self.data.get("trigger_id")) + query_set = QuerySet(TriggerTask).filter(trigger_id=self.data.get("trigger_id")) + if self.data.get("workspace_id"): + query_set = query_set.filter(trigger__workspace_id=self.data.get("workspace_id")) return query_set def list(self, with_valid=True): @@ -70,97 +87,103 @@ class TriggerTaskRecordOperateSerializer(serializers.Serializer): def is_valid(self, *, raise_exception=False): super().is_valid(raise_exception=True) - workspace_id = self.data.get('workspace_id') - query_set = QuerySet(Trigger).filter(id=self.data.get('trigger_id')) + workspace_id = self.data.get("workspace_id") + query_set = QuerySet(Trigger).filter(id=self.data.get("trigger_id")) if workspace_id: query_set = query_set.filter(workspace_id=workspace_id) if not query_set.exists(): - raise AppApiException(500, _('Trigger id does not exist')) + raise AppApiException(500, _("Trigger id does not exist")) def get_execution_details(self, is_valid=True): if is_valid: self.is_valid(raise_exception=True) - task_record = QuerySet(TaskRecord).filter(trigger_id=self.data.get("trigger_id"), - trigger_task_id=self.data.get("trigger_task_id"), - id=self.data.get('trigger_task_record_id')).first() + task_record = ( + QuerySet(TaskRecord) + .filter( + trigger_id=self.data.get("trigger_id"), + trigger_task_id=self.data.get("trigger_task_id"), + id=self.data.get("trigger_task_record_id"), + ) + .first() + ) if not task_record: - raise AppApiException(500, _('Trigger task record id does not exist')) - if task_record.source_type == 'APPLICATION': + raise AppApiException(500, _("Trigger task record id does not exist")) + if task_record.source_type == "APPLICATION": chat_record = QuerySet(ChatRecord).filter(id=task_record.task_record_id).first() if chat_record: return ChatRecordSerializerModel(chat_record).data - return { - 'state': 'TRIGGER_ERROR', - 'meta': task_record.meta - } - if task_record.source_type == 'TOOL': + return {"state": "TRIGGER_ERROR", "meta": task_record.meta} + if task_record.source_type == "TOOL": tool_record = QuerySet(ToolRecord).filter(id=task_record.task_record_id).first() if tool_record: return { - 'id': tool_record.id, - 'tool_id': tool_record.tool_id, - 'workspace_id': tool_record.workspace_id, - 'source_type': tool_record.source_type, - 'source_id': tool_record.source_id, - 'meta': tool_record.meta, - 'state': tool_record.state, - 'run_time': tool_record.run_time, - 'details': { - 'tool_call': { - 'index': 1, - 'result': tool_record.meta.get('output'), - 'params': tool_record.meta.get('input'), - 'status': 500 if tool_record.state == State.FAILURE else 200 if tool_record.state == State.SUCCESS else 201, - 'type': 'tool-node', - 'err_message': tool_record.meta.get('err_message') + "id": tool_record.id, + "tool_id": tool_record.tool_id, + "workspace_id": tool_record.workspace_id, + "source_type": tool_record.source_type, + "source_id": tool_record.source_id, + "meta": tool_record.meta, + "state": tool_record.state, + "run_time": tool_record.run_time, + "details": { + "tool_call": { + "index": 1, + "result": tool_record.meta.get("output"), + "params": tool_record.meta.get("input"), + "status": 500 + if tool_record.state == State.FAILURE + else 200 + if tool_record.state == State.SUCCESS + else 201, + "type": "tool-node", + "err_message": tool_record.meta.get("err_message"), } - } + }, } - return { - 'state': 'TRIGGER_ERROR', - 'meta': task_record.meta - } + return {"state": "TRIGGER_ERROR", "meta": task_record.meta} class TriggerTaskRecordQuerySerializer(serializers.Serializer): trigger_id = serializers.CharField(required=True, label=_("Trigger ID")) workspace_id = serializers.CharField(required=False, allow_null=True, allow_blank=True, label=_("Workspace ID")) - state = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_('Trigger state')) - name = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_('Trigger name')) - source_type = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_('Source type')) - order = serializers.CharField(required=False, allow_null=True, allow_blank=True, label=_('Order field')) + state = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_("Trigger state")) + name = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_("Trigger name")) + source_type = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_("Source type")) + order = serializers.CharField(required=False, allow_null=True, allow_blank=True, label=_("Order field")) def is_valid(self, *, raise_exception=False): super().is_valid(raise_exception=True) - workspace_id = self.data.get('workspace_id') - query_set = QuerySet(Trigger).filter(id=self.data.get('trigger_id')) + workspace_id = self.data.get("workspace_id") + query_set = QuerySet(Trigger).filter(id=self.data.get("trigger_id")) if workspace_id: query_set = query_set.filter(workspace_id=workspace_id) if not query_set.exists(): - raise AppApiException(500, _('Trigger id does not exist')) + raise AppApiException(500, _("Trigger id does not exist")) def get_query_set(self): trigger_query_set = QuerySet( - model=get_dynamics_model({ - 'ett.create_time': models.DateTimeField(), - 'ett.state': models.CharField(), - 'sdc.name': models.CharField(), - 'ett.workspace_id': models.CharField(), - 'ett.trigger_id': models.UUIDField(), - 'sdc.source_type': models.CharField() - })) - trigger_query_set = trigger_query_set.filter( - **{'ett.trigger_id': self.data.get("trigger_id")}) + model=get_dynamics_model( + { + "ett.create_time": models.DateTimeField(), + "ett.state": models.CharField(), + "sdc.name": models.CharField(), + "ett.workspace_id": models.CharField(), + "ett.trigger_id": models.UUIDField(), + "sdc.source_type": models.CharField(), + } + ) + ) + trigger_query_set = trigger_query_set.filter(**{"ett.trigger_id": self.data.get("trigger_id")}) if self.data.get("order"): trigger_query_set = trigger_query_set.order_by(self.data.get("order")) else: trigger_query_set = trigger_query_set.order_by("-ett.create_time") - if self.data.get('state'): - trigger_query_set = trigger_query_set.filter(**{'ett.state': self.data.get('state')}) + if self.data.get("state"): + trigger_query_set = trigger_query_set.filter(**{"ett.state": self.data.get("state")}) if self.data.get("name"): - trigger_query_set = trigger_query_set.filter(**{'sdc.name__contains': self.data.get('name')}) - if self.data.get('source_type'): - trigger_query_set = trigger_query_set.filter(**{'sdc.source_type': self.data.get('source_type')}) + trigger_query_set = trigger_query_set.filter(**{"sdc.name__contains": self.data.get("name")}) + if self.data.get("source_type"): + trigger_query_set = trigger_query_set.filter(**{"sdc.source_type": self.data.get("source_type")}) return trigger_query_set def list(self, with_valid=True): @@ -171,6 +194,11 @@ def list(self, with_valid=True): def page(self, current_page, page_size, with_valid=True): if with_valid: self.is_valid(raise_exception=True) - return native_page_search(current_page, page_size, self.get_query_set(), get_file_content( - os.path.join(PROJECT_DIR, "apps", "trigger", "sql", 'get_trigger_task_record_page_list.sql') - )) + return native_page_search( + current_page, + page_size, + self.get_query_set(), + get_file_content( + os.path.join(PROJECT_DIR, "apps", "trigger", "sql", "get_trigger_task_record_page_list.sql") + ), + ) diff --git a/apps/trigger/tests.py b/apps/trigger/tests.py index 7ce503c2dd9..3e43a07ddba 100644 --- a/apps/trigger/tests.py +++ b/apps/trigger/tests.py @@ -1,3 +1,28 @@ -from django.test import TestCase +from unittest.mock import MagicMock, call, patch -# Create your tests here. +from django.test import SimpleTestCase + +from trigger.serializers.trigger_task import TriggerTaskQuerySerializer + + +class TriggerTaskQueryTests(SimpleTestCase): + def test_task_query_filters_workspace_through_trigger_relation(self): + query_set = MagicMock() + query_set.filter.return_value = query_set + query_set.exists.return_value = True + + with patch("trigger.serializers.trigger_task.QuerySet", side_effect=lambda model: query_set): + serializer = TriggerTaskQuerySerializer(data={"trigger_id": "trigger-1", "workspace_id": "workspace-1"}) + serializer.is_valid(raise_exception=True) + serializer.get_query_set() + + query_set.filter.assert_has_calls( + [ + call(trigger_id="trigger-1"), + call(trigger__workspace_id="workspace-1"), + ] + ) + self.assertNotIn( + call(workspace_id="workspace-1", trigger_id="trigger-1"), + query_set.filter.call_args_list, + ) diff --git a/apps/users/serializers/user.py b/apps/users/serializers/user.py index 0bcd39e084d..51f3c765b70 100644 --- a/apps/users/serializers/user.py +++ b/apps/users/serializers/user.py @@ -645,7 +645,8 @@ def edit(self, instance, user_id, with_valid=True): self._check_admin_modification(user, instance) self._update_user_fields(user, instance) update_user_role(instance, user, user_id) - set_user_groups(user.id, instance) + if "user_group_ids" in instance: + set_user_groups(user.id, instance) user.save() return UserInstanceSerializer(user).data diff --git a/apps/users/serializers/user_group.py b/apps/users/serializers/user_group.py index a9f5c873d71..70212566f2b 100644 --- a/apps/users/serializers/user_group.py +++ b/apps/users/serializers/user_group.py @@ -10,7 +10,7 @@ from common.auth.constants.role_constants import RoleConstants from common.database_model_manage.database_model_manage import DatabaseModelManage from common.db.search import page_search -from common.exception.app_exception import AppApiException +from common.exception.app_exception import AppApiException, NotFound404 from system_manage.models import UserGroup, UserGroupRelation from users.models.user_group import SystemUserGroup, SystemUserGroupRelation @@ -227,7 +227,7 @@ def is_valid(self, *, raise_exception=False): group_id = self.data.get("group_id") workspace_id = self.data.get("workspace_id") if not SystemUserGroup.objects.filter(id=group_id, workspace_id=workspace_id).exists(): - raise AppApiException(500, _("User group does not exist")) + raise NotFound404(404, _("User group does not exist")) def page(self, current_page, page_size): self.is_valid() diff --git a/apps/users/tests/test_user_group_workspace_validation.py b/apps/users/tests/test_user_group_workspace_validation.py new file mode 100644 index 00000000000..e63a2e48a0c --- /dev/null +++ b/apps/users/tests/test_user_group_workspace_validation.py @@ -0,0 +1,22 @@ +from unittest.mock import MagicMock, patch + +from django.test import SimpleTestCase + +from common.exception.app_exception import NotFound404 +from users.serializers.user_group import UserGroupListPageSerializer + + +class UserGroupWorkspaceValidationTests(SimpleTestCase): + def test_missing_or_cross_workspace_group_returns_not_found(self): + query_set = MagicMock() + query_set.exists.return_value = False + serializer = UserGroupListPageSerializer.Query(data={"workspace_id": "workspace-1", "group_id": "group-1"}) + + with ( + patch("users.serializers.user_group.SystemUserGroup.objects.filter", return_value=query_set), + self.assertRaises(NotFound404) as context, + ): + serializer.is_valid(raise_exception=True) + + self.assertEqual(404, context.exception.status_code) + self.assertNotIn("does not exist or does not belong", str(context.exception.message)) diff --git a/apps/users/tests/test_user_management.py b/apps/users/tests/test_user_management.py new file mode 100644 index 00000000000..a340a30c18a --- /dev/null +++ b/apps/users/tests/test_user_management.py @@ -0,0 +1,25 @@ +from django.test import TestCase + +from users.models import User +from users.models.user_group import SystemUserGroup, SystemUserGroupRelation +from users.serializers.user import UserManageSerializer + + +class UserManagementGroupPreservationTests(TestCase): + def test_status_and_password_changes_preserve_user_groups(self): + user = User.objects.create( + username="system-user", + nick_name="System User", + password="encrypted-password", + role="USER", + source="LOCAL", + ) + group = SystemUserGroup.objects.create(name="system-user-group", workspace_id="default") + SystemUserGroupRelation.objects.create(user=user, group=group) + operate = UserManageSerializer.Operate(data={"id": user.id}) + + operate.edit({"is_active": False}, user.id, with_valid=True) + operate.edit({"is_active": True}, user.id, with_valid=True) + operate.re_password({"password": "Password1!", "re_password": "Password1!"}, with_valid=True) + + self.assertTrue(SystemUserGroupRelation.objects.filter(user=user, group=group).exists()) diff --git a/ui/src/views/system/chat-management/user-groups/index.vue b/ui/src/views/system/chat-management/user-groups/index.vue index e3e1dc7d674..225e62d1962 100644 --- a/ui/src/views/system/chat-management/user-groups/index.vue +++ b/ui/src/views/system/chat-management/user-groups/index.vue @@ -124,13 +124,17 @@ function handleBatchSelectionChange(selection: unknown[]) { } function handleRemoveMembers(member?: ChatUserGroupMember) { + const group = currentGroup.value + const members = member ? [member] : batchSelectedMembers.value + if (!group || !members.length) return + const targetName = member ? member.nick_name || member.username : '' - const title = member ? `是否移除用户：${targetName}？` : `是否移除选中的 ${batchSelectedMembers.value.length} 个用户？` + const title = member ? `是否移除用户：${targetName}？` : `是否移除选中的 ${members.length} 个用户？` MsgConfirm(title, '成员移除后，通过该用户组继承的资源权限将被回收，请谨慎操作。', { confirmButtonText: '移除' }) .then(() => { membersLoading.value = true - const relationIds = batchSelectedMembers.value.map(({ user_group_relation_id }) => user_group_relation_id) - return ChatGroupsApi.postRemoveChatUserGroupMembers(currentGroup.value!.id, relationIds).then(async () => { + const relationIds = members.map(({ user_group_relation_id }) => user_group_relation_id) + return ChatGroupsApi.postRemoveChatUserGroupMembers(group.id, relationIds).then(async () => { MsgSuccess('移除成功') await loadGroupMembers() memberTableRef.value?.clearSelection() diff --git a/ui/src/views/system/chat-management/users/index.vue b/ui/src/views/system/chat-management/users/index.vue index 2ac6f992e72..8be7f28963d 100644 --- a/ui/src/views/system/chat-management/users/index.vue +++ b/ui/src/views/system/chat-management/users/index.vue @@ -200,7 +200,7 @@ onMounted(() => loadChatUsers()) - +