From dfd9282a1a2f37a7773f2cdf1205b017bbf77e09 Mon Sep 17 00:00:00 2001 From: nannan00 <17491932+nannan00@users.noreply.github.com> Date: Thu, 26 Oct 2023 11:10:39 +0800 Subject: [PATCH] refactor(bklogin): plugin exception --- .../bklogin/authentication/api_views.py | 2 +- .../authentication/{helper.py => manager.py} | 0 src/bk-login/bklogin/authentication/urls.py | 1 + src/bk-login/bklogin/authentication/views.py | 80 ++++++++++++++----- src/bk-login/bklogin/common/error_codes.py | 6 ++ src/idp-plugins/idp_plugins/exceptions.py | 8 ++ .../idp_plugins/local/exceptions.py | 27 ------- .../idp_plugins/local/implement.py | 10 +-- src/idp-plugins/idp_plugins/wecom/client.py | 5 +- .../idp_plugins/wecom/exceptions.py | 23 ------ .../idp_plugins/wecom/implement.py | 2 +- 11 files changed, 83 insertions(+), 81 deletions(-) rename src/bk-login/bklogin/authentication/{helper.py => manager.py} (100%) delete mode 100644 src/idp-plugins/idp_plugins/local/exceptions.py delete mode 100644 src/idp-plugins/idp_plugins/wecom/exceptions.py diff --git a/src/bk-login/bklogin/authentication/api_views.py b/src/bk-login/bklogin/authentication/api_views.py index 9d322ec08..39647dca6 100644 --- a/src/bk-login/bklogin/authentication/api_views.py +++ b/src/bk-login/bklogin/authentication/api_views.py @@ -16,7 +16,7 @@ from bklogin.common.error_codes import error_codes from bklogin.common.response import APISuccessResponse -from .helper import BkTokenManager +from .manager import BkTokenManager class CheckTokenApi(View): diff --git a/src/bk-login/bklogin/authentication/helper.py b/src/bk-login/bklogin/authentication/manager.py similarity index 100% rename from src/bk-login/bklogin/authentication/helper.py rename to src/bk-login/bklogin/authentication/manager.py diff --git a/src/bk-login/bklogin/authentication/urls.py b/src/bk-login/bklogin/authentication/urls.py index e2c3bed37..5aefdacba 100644 --- a/src/bk-login/bklogin/authentication/urls.py +++ b/src/bk-login/bklogin/authentication/urls.py @@ -40,6 +40,7 @@ # OpenAPI urlpatterns += [ + # FIXME: 临时兼容，OpenAPI后面接入APIGateway, 还要考虑兼容原有通过ESB和直接调用的 path("api/v1/is_login/", api_views.CheckTokenApi.as_view()), path("api/v1/get_user/", api_views.GetUserApi.as_view()), ] diff --git a/src/bk-login/bklogin/authentication/views.py b/src/bk-login/bklogin/authentication/views.py index bb77b8a50..fc36b5c0e 100644 --- a/src/bk-login/bklogin/authentication/views.py +++ b/src/bk-login/bklogin/authentication/views.py @@ -8,9 +8,11 @@ an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. """ -from typing import Any, Dict, List +import logging +from typing import Any, Callable, Dict, List from urllib.parse import quote_plus, urljoin +import pydantic from django.conf import settings from django.http import HttpResponseNotFound, HttpResponseRedirect from django.shortcuts import render @@ -28,9 +30,10 @@ from bklogin.common.response import APISuccessResponse from bklogin.idp_plugins.base import BaseCredentialIdpPlugin, BaseFederationIdpPlugin, get_plugin_cls from bklogin.idp_plugins.constants import AllowedHttpMethodEnum, BuiltinActionEnum +from bklogin.idp_plugins.exceptions import InvalidParamError, ParseRequestBodyError, UnexpectedDataError from .constants import ALLOWED_SIGN_IN_TENANT_USER_IDS_SESSION_KEY, REDIRECT_FIELD_NAME, SIGN_IN_TENANT_ID_SESSION_KEY -from .helper import BkTokenManager +from .manager import BkTokenManager # 确保无论何时，响应必然有CSRFToken Cookie @@ -175,6 +178,14 @@ def get(self, request, *args, **kwargs): return APISuccessResponse(data=data) +class PluginErrorContext(pydantic.BaseModel): + """插件异常上下文，用于打印日志时所需的上下文信息""" + + idp: Idp + action: str + http_method: str + + # 先对所有请求豁免CSRF校验，由dispatch里根据需要手动执行CSRF校验 @method_decorator(csrf_exempt, name="dispatch") class IdpPluginDispatchView(View): @@ -200,8 +211,8 @@ def dispatch(self, request, *args, **kwargs): # (1) 获取插件 try: plugin_cls = get_plugin_cls(idp.plugin_id) - except Exception as error: - raise error_codes.SYSTEM_ERROR.f( + except NotImplementedError as error: + raise error_codes.PLUGIN_SYSTEM_ERROR.f( _("认证源[{}]获取插件[{}]失败, {}").format(idp.name, idp.plugin.name, error), ) @@ -209,8 +220,15 @@ def dispatch(self, request, *args, **kwargs): try: plugin_cfg = plugin_cls.config_class(**idp.plugin_config) plugin = plugin_cls(cfg=plugin_cfg) + except pydantic.ValidationError: + logging.exception("idp(%s) init plugin(%s) config failed", idp.id, idp.plugin.id) + # Note: 不可将error对外，因为配置信息比较敏感 + raise error_codes.PLUGIN_SYSTEM_ERROR.f( + _("认证源[{}]初始化插件配置[{}]失败").format(idp.name, idp.plugin.name), + ) except Exception as error: - raise error_codes.SYSTEM_ERROR.f( + logging.exception("idp(%s) load plugin(%s) failed", idp.id, idp.plugin.id) + raise error_codes.PLUGIN_SYSTEM_ERROR.f( _("认证源[{}]加载插件[{}]失败, {}").format(idp.name, idp.plugin.name, error), ) @@ -226,6 +244,28 @@ def dispatch(self, request, *args, **kwargs): return HttpResponseNotFound() + def wrap_plugin_error(self, context: PluginErrorContext, func: Callable, *func_args, **func_kwargs): + """统一捕获插件异常""" + try: + return func(*func_args, **func_kwargs) + except ParseRequestBodyError as e: + raise error_codes.INVALID_ARGUMENT.f(str(e)) + except InvalidParamError as e: + raise error_codes.VALIDATION_ERROR.f(str(e)) + except UnexpectedDataError as e: + raise error_codes.UNEXPECTED_DATA_ERROR.f(str(e)) + except Exception: + logging.exception( + "idp(%s) request failed, when dispatch (%s, %s) to credential idp plugin(%s)", + context.idp.id, + context.action, + context.http_method, + context.idp.plugin.id, + ) + raise error_codes.PLUGIN_SYSTEM_ERROR.f( + _("认证源[{}]执行插件[{}]失败, {}").format(context.idp.name, context.idp.plugin.name), + ) + def _dispatch_credential_idp_plugin( self, plugin: BaseCredentialIdpPlugin, request, sign_in_tenant_id: str, idp: Idp, action: str, http_method: str ): @@ -233,14 +273,11 @@ def _dispatch_credential_idp_plugin( 身份凭证类的插件执行请求分配 """ dispatch_cfs = (action, http_method) + plugin_error_context = PluginErrorContext(idp=idp, action=action, http_method=http_method) # 对凭证进行认证 if dispatch_cfs == (BuiltinActionEnum.AUTHENTICATE, AllowedHttpMethodEnum.POST): # 认证 - try: - user_infos = plugin.authenticate_credentials(request) - except ValueError as error: - # FIXME: 应该定义插件类的异常，某些异常是参数错误，某些异常是系统问题, 下同 - raise error_codes.VALIDATION_ERROR.f(str(error)) + user_infos = self.wrap_plugin_error(plugin_error_context, plugin.authenticate_credentials, request=request) # 使用认证源获得的用户信息，匹配认证出对应的租户用户列表 tenant_user_ids = self._auth_backend(request, sign_in_tenant_id, idp, user_infos) @@ -249,7 +286,9 @@ def _dispatch_credential_idp_plugin( # 身份凭证认证直接返回成功即可，由前端重定向路由到用户列表选择页面 return APISuccessResponse() - return plugin.dispatch_extension(action, http_method, request) + return self.wrap_plugin_error( + plugin_error_context, plugin.dispatch_extension, action=action, http_method=http_method, request=request + ) def _dispatch_federation_idp_plugin( self, plugin: BaseFederationIdpPlugin, request, sign_in_tenant_id: str, idp: Idp, action: str, http_method: str @@ -258,13 +297,13 @@ def _dispatch_federation_idp_plugin( 联邦认证类的插件执行请求分配 """ dispatch_cfs = (action, http_method) + plugin_error_context = PluginErrorContext(idp=idp, action=action, http_method=http_method) # 跳转到第三方登录 if dispatch_cfs == (BuiltinActionEnum.LOGIN, AllowedHttpMethodEnum.GET): - try: - callback_uri = self._get_complete_action_url(idp.id, BuiltinActionEnum.CALLBACK) - redirect_uri = plugin.build_login_uri(request, callback_uri) - except ValueError as error: - raise error_codes.VALIDATION_ERROR.f(str(error)) + callback_uri = self._get_complete_action_url(idp.id, BuiltinActionEnum.CALLBACK) + redirect_uri = self.wrap_plugin_error( + plugin_error_context, plugin.build_login_uri, request=request, callback_uri=callback_uri + ) return HttpResponseRedirect(redirect_uri) @@ -276,10 +315,7 @@ def _dispatch_federation_idp_plugin( (BuiltinActionEnum.CALLBACK, AllowedHttpMethodEnum.POST), ]: # 认证 - try: - user_info = plugin.handle_callback(request) - except ValueError as error: - raise error_codes.VALIDATION_ERROR.f(str(error)) + user_info = self.wrap_plugin_error(plugin_error_context, plugin.handle_callback, request=request) # 使用认证源获得的用户信息，匹配认证出对应的租户用户列表 tenant_user_ids = self._auth_backend(request, sign_in_tenant_id, idp, user_info) @@ -288,7 +324,9 @@ def _dispatch_federation_idp_plugin( # 联邦认证则重定向到前端选择账号页面 return HttpResponseRedirect(redirect_to="pages/users") - return plugin.dispatch_extension(action, http_method, request) + return self.wrap_plugin_error( + plugin_error_context, plugin.dispatch_extension, action=action, http_method=http_method, request=request + ) def _get_complete_action_url(self, idp_id: str, action: str) -> str: """获取完整""" diff --git a/src/bk-login/bklogin/common/error_codes.py b/src/bk-login/bklogin/common/error_codes.py index be9fc1278..201a7d14c 100644 --- a/src/bk-login/bklogin/common/error_codes.py +++ b/src/bk-login/bklogin/common/error_codes.py @@ -56,6 +56,12 @@ class ErrorCodes: status_code=HTTPStatus.NOT_FOUND, ) VALIDATION_ERROR = ErrorCode(_("参数校验不通过")) + UNEXPECTED_DATA_ERROR = ErrorCode(_("非预期数据")) + PLUGIN_SYSTEM_ERROR = ErrorCode( + _("插件执行异常"), + code_category=ErrorCodeCategoryEnum.INTERNAL, + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + ) SYSTEM_ERROR = ErrorCode( _("系统异常"), code_category=ErrorCodeCategoryEnum.INTERNAL, diff --git a/src/idp-plugins/idp_plugins/exceptions.py b/src/idp-plugins/idp_plugins/exceptions.py index b9c76661f..05ef099aa 100644 --- a/src/idp-plugins/idp_plugins/exceptions.py +++ b/src/idp-plugins/idp_plugins/exceptions.py @@ -20,3 +20,11 @@ class ParseRequestBodyError(IdpPluginBaseError): class RequestAPIError(IdpPluginBaseError): """请求第三方接口失败""" + + +class InvalidParamError(IdpPluginBaseError): + """参数非合法""" + + +class UnexpectedDataError(IdpPluginBaseError): + """数据非预期""" diff --git a/src/idp-plugins/idp_plugins/local/exceptions.py b/src/idp-plugins/idp_plugins/local/exceptions.py deleted file mode 100644 index a6df9c9d0..000000000 --- a/src/idp-plugins/idp_plugins/local/exceptions.py +++ /dev/null @@ -1,27 +0,0 @@ -# -*- coding: utf-8 -*- -""" -TencentBlueKing is pleased to support the open source community by making 蓝鲸智云-用户管理(Bk-User) available. -Copyright (C) 2017-2021 THL A29 Limited, a Tencent company. All rights reserved. -Licensed under the MIT License (the "License"); you may not use this file except in compliance with the License. -You may obtain a copy of the License at http://opensource.org/licenses/MIT -Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on -an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the -specific language governing permissions and limitations under the License. -""" -from ..exceptions import IdpPluginBaseError - - -class LocalIdpPluginBaseError(IdpPluginBaseError): - """本地认证源异常基类""" - - -class UserCredentialRequiredError(LocalIdpPluginBaseError): - """用户凭证-用户名或密码为空""" - - -class NotEnabledPasswordLoginError(LocalIdpPluginBaseError): - """没有数据源启用账密登录""" - - -class UserCredentialIncorrectError(LocalIdpPluginBaseError): - """用户凭证 - 用户名或命名不正确""" diff --git a/src/idp-plugins/idp_plugins/local/implement.py b/src/idp-plugins/idp_plugins/local/implement.py index 37b7ed38f..2972da1e8 100644 --- a/src/idp-plugins/idp_plugins/local/implement.py +++ b/src/idp-plugins/idp_plugins/local/implement.py @@ -15,7 +15,7 @@ from pydantic import BaseModel from .db_models import LocalDataSourceIdentityInfo -from .exceptions import NotEnabledPasswordLoginError, UserCredentialIncorrectError, UserCredentialRequiredError +from ..exceptions import InvalidParamError, UnexpectedDataError from ..base import BaseCredentialIdpPlugin from ..models import TestConnectionResult from ..tools import parse_request_body_json @@ -46,14 +46,14 @@ def authenticate_credentials(self, request: HttpRequest) -> List[Dict[str, Any]] username = request_body.get("username") if not username: - raise UserCredentialRequiredError(_("用户名不允许为空")) + raise InvalidParamError(_("用户名不允许为空")) password = request_body.get("password") if not password: - raise UserCredentialRequiredError(_("密码不允许为空")) + raise InvalidParamError(_("密码不允许为空")) if not self.cfg.data_source_ids: - raise NotEnabledPasswordLoginError(_("当前租户没有数据源允许账密登录")) + raise UnexpectedDataError(_("当前租户没有数据源允许账密登录")) # FIXME (nan): 待用户密码功能改造完成后重新调整校验密码方式 users = LocalDataSourceIdentityInfo.objects.filter( @@ -65,6 +65,6 @@ def authenticate_credentials(self, request: HttpRequest) -> List[Dict[str, Any]] # 判断是否有用户匹配 if len(matched_users) == 0: - raise UserCredentialIncorrectError(_("用户名或密码不正确")) + raise InvalidParamError(_("用户名或密码不正确")) return [{"id": i.user.id} for i in matched_users] diff --git a/src/idp-plugins/idp_plugins/wecom/client.py b/src/idp-plugins/idp_plugins/wecom/client.py index 8db600edd..0fb62ce83 100644 --- a/src/idp-plugins/idp_plugins/wecom/client.py +++ b/src/idp-plugins/idp_plugins/wecom/client.py @@ -14,9 +14,8 @@ from django.utils.translation import gettext_lazy as _ -from .exceptions import NotCorpMemberError from .settings import WECOM_API_BASE_URL -from ..exceptions import RequestAPIError +from ..exceptions import RequestAPIError, UnexpectedDataError from ..http import http_get logger = logging.getLogger(__name__) @@ -97,4 +96,4 @@ def get_user_id_by_code(self, code: str) -> str: if userid: return userid - raise NotCorpMemberError(_("非企业成员，登录认证失败")) + raise UnexpectedDataError(_("非企业成员，登录认证失败")) diff --git a/src/idp-plugins/idp_plugins/wecom/exceptions.py b/src/idp-plugins/idp_plugins/wecom/exceptions.py deleted file mode 100644 index 3d67bb46c..000000000 --- a/src/idp-plugins/idp_plugins/wecom/exceptions.py +++ /dev/null @@ -1,23 +0,0 @@ -# -*- coding: utf-8 -*- -""" -TencentBlueKing is pleased to support the open source community by making 蓝鲸智云-用户管理(Bk-User) available. -Copyright (C) 2017-2021 THL A29 Limited, a Tencent company. All rights reserved. -Licensed under the MIT License (the "License"); you may not use this file except in compliance with the License. -You may obtain a copy of the License at http://opensource.org/licenses/MIT -Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on -an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the -specific language governing permissions and limitations under the License. -""" -from ..exceptions import IdpPluginBaseError - - -class WecomIdpPluginBaseError(IdpPluginBaseError): - """企业微信认证源插件异常基类""" - - -class InvalidParamError(WecomIdpPluginBaseError): - """参数非合法""" - - -class NotCorpMemberError(WecomIdpPluginBaseError): - """非企业成员""" diff --git a/src/idp-plugins/idp_plugins/wecom/implement.py b/src/idp-plugins/idp_plugins/wecom/implement.py index 688a042b1..757694154 100644 --- a/src/idp-plugins/idp_plugins/wecom/implement.py +++ b/src/idp-plugins/idp_plugins/wecom/implement.py @@ -16,7 +16,7 @@ from pydantic import BaseModel from .client import WeComAPIClient -from .exceptions import InvalidParamError +from ..exceptions import InvalidParamError from .settings import WECOM_OAUTH_URL from ..base import BaseFederationIdpPlugin from ..models import TestConnectionResult