From cb6a6b87c9335e892ac55d50e49d35957f9d45b8 Mon Sep 17 00:00:00 2001 From: YANG QIA <2013xile@gmail.com> Date: Fri, 15 Dec 2023 21:03:16 +0800 Subject: [PATCH] fix(acl): optimize error handling when logged user has no roles (#3190) * fix: fix T-2720 * fix: localization * fix: test * fix: build * chore: update * fix: update title * chore: update title * fix: app load error * fix: load error * fix: test error --------- Co-authored-by: chenos --- .../client/src/application/Application.tsx | 10 ++++- .../client/src/application/PluginManager.ts | 27 ++++-------- .../client/src/application/WebSocketClient.ts | 2 +- .../application/components/AppComponent.tsx | 8 ++-- .../components/defaultComponents.tsx | 2 +- packages/core/client/src/auth/SigninPage.tsx | 8 +++- packages/core/client/src/locale/en_US.json | 6 ++- packages/core/client/src/locale/zh-CN.json | 6 ++- .../src/nocobase-buildin-plugin/index.tsx | 43 ++++++++++++++++++- .../plugins/LocalePlugin.ts | 7 ++- packages/core/database/src/repository.ts | 1 - packages/core/server/src/locale/locale.ts | 1 + .../plugin-acl/src/locale/en-US.json | 4 ++ .../plugin-acl/src/locale/zh-CN.json | 4 ++ .../server/__tests__/setCurrentRole.test.ts | 11 ++++- .../src/server/__tests__/users.test.ts | 2 +- .../src/server/middlewares/setCurrentRole.ts | 12 +++++- 17 files changed, 117 insertions(+), 37 deletions(-) create mode 100644 packages/plugins/@nocobase/plugin-acl/src/locale/en-US.json create mode 100644 packages/plugins/@nocobase/plugin-acl/src/locale/zh-CN.json diff --git a/packages/core/client/src/application/Application.tsx b/packages/core/client/src/application/Application.tsx index ec8975838..afa3abf60 100644 --- a/packages/core/client/src/application/Application.tsx +++ b/packages/core/client/src/application/Application.tsx @@ -192,9 +192,10 @@ export class Application { this.maintaining = true; this.error = data.payload; } else { - console.log('loadFailed', loadFailed); + // console.log('loadFailed', loadFailed); if (loadFailed) { window.location.reload(); + return; } this.maintaining = false; this.maintained = true; @@ -203,14 +204,21 @@ export class Application { }); this.ws.on('serverDown', () => { this.maintaining = true; + this.maintained = false; }); this.ws.connect(); try { this.loading = true; await this.pm.load(); } catch (error) { + if (this.ws.enabled) { + await new Promise((resolve) => { + setTimeout(() => resolve(null), 1000); + }); + } loadFailed = true; this.error = { + ...error, code: 'LOAD_ERROR', message: error.message, }; diff --git a/packages/core/client/src/application/PluginManager.ts b/packages/core/client/src/application/PluginManager.ts index 740713f91..f020040dc 100644 --- a/packages/core/client/src/application/PluginManager.ts +++ b/packages/core/client/src/application/PluginManager.ts @@ -42,24 +42,15 @@ export class PluginManager { } private async initRemotePlugins() { - try { - const res = await this.app.apiClient.request({ url: 'pm:listEnabled' }); - const pluginList: PluginData[] = res?.data?.data || []; - const plugins = await getPlugins({ - requirejs: this.app.requirejs, - pluginData: pluginList, - devDynamicImport: this.app.devDynamicImport, - }); - for await (const [name, pluginClass] of plugins) { - await this.add(pluginClass, { name }); - } - } catch (error) { - if (401 === error?.response?.status) { - this.app.apiClient.auth.setRole(null); - window.location.reload(); - } else { - throw error; - } + const res = await this.app.apiClient.request({ url: 'pm:listEnabled' }); + const pluginList: PluginData[] = res?.data?.data || []; + const plugins = await getPlugins({ + requirejs: this.app.requirejs, + pluginData: pluginList, + devDynamicImport: this.app.devDynamicImport, + }); + for await (const [name, pluginClass] of plugins) { + await this.add(pluginClass, { name }); } } diff --git a/packages/core/client/src/application/WebSocketClient.ts b/packages/core/client/src/application/WebSocketClient.ts index b730e5865..3ce776f6e 100644 --- a/packages/core/client/src/application/WebSocketClient.ts +++ b/packages/core/client/src/application/WebSocketClient.ts @@ -36,7 +36,7 @@ export class WebSocketClient { protected _reconnectTimes = 0; protected events = []; protected options: WebSocketClientOptions; - protected enabled: boolean; + enabled: boolean; connected = false; serverDown = false; lastMessage = {}; diff --git a/packages/core/client/src/application/components/AppComponent.tsx b/packages/core/client/src/application/components/AppComponent.tsx index 0b00dd934..0c0382b9e 100644 --- a/packages/core/client/src/application/components/AppComponent.tsx +++ b/packages/core/client/src/application/components/AppComponent.tsx @@ -1,6 +1,6 @@ import { observer } from '@formily/reactive-react'; import React, { FC, useCallback, useEffect } from 'react'; -import { ErrorBoundary, FallbackProps } from 'react-error-boundary'; +import { ErrorBoundary } from 'react-error-boundary'; import type { Application } from '../Application'; import { ApplicationContext } from '../context'; @@ -13,7 +13,6 @@ export const AppComponent: FC = observer((props) => { const handleErrors = useCallback((error: Error, info: { componentStack: string }) => { console.error(error, info); }, []); - useEffect(() => { app.load(); }, [app]); @@ -22,7 +21,10 @@ export const AppComponent: FC = observer((props) => { if (!app.maintained && app.maintaining) return app.renderComponent('AppMaintaining', { app }); if (app.error?.code === 'LOAD_ERROR') return ; return ( - } onError={handleErrors}> + } + onError={handleErrors} + > {app.maintained && app.maintaining && app.renderComponent('AppMaintainingDialog', { app })} {app.renderComponent('AppMain')} diff --git a/packages/core/client/src/application/components/defaultComponents.tsx b/packages/core/client/src/application/components/defaultComponents.tsx index 38b1884f7..7ddcb8d98 100644 --- a/packages/core/client/src/application/components/defaultComponents.tsx +++ b/packages/core/client/src/application/components/defaultComponents.tsx @@ -9,7 +9,7 @@ const AppError: FC<{ error: Error }> = ({ error }) => ( ); -const AppNotFound: FC = () =>
Not Found
; +const AppNotFound: FC = () =>
; export const defaultAppComponents = { AppMain: MainComponent, diff --git a/packages/core/client/src/auth/SigninPage.tsx b/packages/core/client/src/auth/SigninPage.tsx index 264bec23e..d7e938383 100644 --- a/packages/core/client/src/auth/SigninPage.tsx +++ b/packages/core/client/src/auth/SigninPage.tsx @@ -12,7 +12,7 @@ import React, { } from 'react'; import { useTranslation } from 'react-i18next'; import { useNavigate, useSearchParams } from 'react-router-dom'; -import { useAPIClient, useCurrentDocumentTitle, useCurrentUserContext, useRequest, useViewport } from '..'; +import { useAPIClient, useApp, useCurrentDocumentTitle, useCurrentUserContext, useRequest, useViewport } from '..'; import { useSigninPageExtension } from './SigninPageExtension'; const SigninPageContext = createContext<{ @@ -106,7 +106,7 @@ export const SigninPage = () => { setTabs(tabs); }; - useRequest( + const { error } = useRequest( () => api .resource('authenticators') @@ -121,6 +121,10 @@ export const SigninPage = () => { }, ); + if (error) { + throw error; + } + if (!authenticators.length) { return
{t('Oops! No authentication methods available.')}
; } diff --git a/packages/core/client/src/locale/en_US.json b/packages/core/client/src/locale/en_US.json index f1c95259f..ac0ef54ec 100644 --- a/packages/core/client/src/locale/en_US.json +++ b/packages/core/client/src/locale/en_US.json @@ -736,6 +736,7 @@ "Enable link": "Enable link", "This is likely a NocoBase internals bug. Please open an issue at <1>here": "This is likely a NocoBase internals bug. Please open an issue at <1>here", "Render Failed": "Render Failed", + "App error": "App error", "Feedback": "Feedback", "Try again": "Try again", "Data template": "Data template", @@ -772,5 +773,8 @@ "Execute": "Execute", "Please use a valid SELECT or WITH AS statement": "Please use a valid SELECT or WITH AS statement", "Please confirm the SQL statement first": "Please confirm the SQL statement first", - "Automatically drop objects that depend on the collection (such as views), and in turn all objects that depend on those objects": "Automatically drop objects that depend on the collection (such as views), and in turn all objects that depend on those objects" + "Automatically drop objects that depend on the collection (such as views), and in turn all objects that depend on those objects": "Automatically drop objects that depend on the collection (such as views), and in turn all objects that depend on those objects", + "Sign in with another account": "Sign in with another account", + "Return to the main application": "Return to the main application", + "Permission deined": "Permission denied" } diff --git a/packages/core/client/src/locale/zh-CN.json b/packages/core/client/src/locale/zh-CN.json index 28211e18e..f2c0ca190 100644 --- a/packages/core/client/src/locale/zh-CN.json +++ b/packages/core/client/src/locale/zh-CN.json @@ -812,6 +812,7 @@ "Direct duplicate": "直接复制", "Copy into the form and continue to fill in": "复制到表单并继续填写", "Linkage with form fields": "从表单字段联动", + "App error": "应用错误", "Failed to load plugin": "插件加载失败", "Allow add new, update and delete actions": "允许增删改操作", "Date display format": "日期显示格式", @@ -828,5 +829,8 @@ "Execute": "执行", "Please use a valid SELECT or WITH AS statement": "请使用有效的 SELECT 或 WITH AS 语句", "Please confirm the SQL statement first": "请先确认 SQL 语句", - "Automatically drop objects that depend on the collection (such as views), and in turn all objects that depend on those objects": "自动删除依赖于该表的对象,以及依赖这些对象的对象" + "Automatically drop objects that depend on the collection (such as views), and in turn all objects that depend on those objects": "自动删除依赖于该表的对象,以及依赖这些对象的对象", + "Sign in with another account": "登录其他账号", + "Return to the main application": "返回主应用", + "Permission denied": "没有权限" } diff --git a/packages/core/client/src/nocobase-buildin-plugin/index.tsx b/packages/core/client/src/nocobase-buildin-plugin/index.tsx index 331c4e47f..5442bb1ab 100644 --- a/packages/core/client/src/nocobase-buildin-plugin/index.tsx +++ b/packages/core/client/src/nocobase-buildin-plugin/index.tsx @@ -1,11 +1,13 @@ import { DisconnectOutlined, LoadingOutlined } from '@ant-design/icons'; import { css } from '@emotion/css'; import { observer } from '@formily/reactive-react'; +import { getSubAppName } from '@nocobase/sdk'; import { Button, Modal, Result, Spin } from 'antd'; import React, { FC } from 'react'; import { Navigate, useNavigate } from 'react-router-dom'; import { ACLPlugin } from '../acl'; -import { Application, useApp } from '../application'; +import { useAPIClient } from '../api-client'; +import { Application } from '../application'; import { Plugin } from '../application/Plugin'; import { SigninPage, SigninPageExtensionPlugin, SignupPage } from '../auth'; import { BlockSchemaComponentPlugin } from '../block-provider'; @@ -32,7 +34,43 @@ const AppSpin = () => { ); }; +const useErrorProps = (app: Application, error: any) => { + const api = useAPIClient(); + if (!error) { + return {}; + } + const err = error?.response?.data?.errors?.[0] || error; + const subApp = getSubAppName(); + switch (err.code) { + case 'USER_HAS_NO_ROLES_ERR': + return { + title: app.i18n.t('Permission denied'), + subTitle: err.message, + extra: [ + , + subApp ? ( + + ) : null, + ], + }; + default: + return {}; + } +}; + const AppError: FC<{ error: Error; app: Application }> = observer(({ app, error }) => { + const props = useErrorProps(app, error); return (
= observer(({ app, error transform: translate(0, -50%); `} status="error" - title={app.i18n.t('Failed to load plugin')} + title={app.i18n.t('App error')} subTitle={app.i18n.t(error?.message)} extra={[ , ]} + {...props} />
); diff --git a/packages/core/client/src/nocobase-buildin-plugin/plugins/LocalePlugin.ts b/packages/core/client/src/nocobase-buildin-plugin/plugins/LocalePlugin.ts index be904219c..2af3ce323 100644 --- a/packages/core/client/src/nocobase-buildin-plugin/plugins/LocalePlugin.ts +++ b/packages/core/client/src/nocobase-buildin-plugin/plugins/LocalePlugin.ts @@ -1,9 +1,9 @@ +import { setValidateLanguage } from '@formily/validator'; import { App, ConfigProvider } from 'antd'; import dayjs from 'dayjs'; import { loadConstrueLocale } from '../../antd-config-provider/loadConstrueLocale'; import { Plugin } from '../../application/Plugin'; import { dayjsLocale } from '../../locale'; -import { setValidateLanguage } from '@formily/validator'; export class LocalePlugin extends Plugin { locales: any = {}; @@ -16,12 +16,15 @@ export class LocalePlugin extends Plugin { params: { locale, }, + headers: { + 'X-Role': 'anonymous', + }, }); const data = res?.data; this.locales = data?.data || {}; this.app.use(ConfigProvider, { locale: this.locales.antd, popupMatchSelectWidth: false }); this.app.use(App); - if (data?.data?.lang && !locale) { + if (data?.data?.lang) { api.auth.setLocale(data?.data?.lang); this.app.i18n.changeLanguage(data?.data?.lang); } diff --git a/packages/core/database/src/repository.ts b/packages/core/database/src/repository.ts index e67ceef4d..8e2ac2527 100644 --- a/packages/core/database/src/repository.ts +++ b/packages/core/database/src/repository.ts @@ -479,7 +479,6 @@ export class Repository { this.app.i18n.addResources(lang, name, resources[name]); }); + return resources; } diff --git a/packages/plugins/@nocobase/plugin-acl/src/locale/en-US.json b/packages/plugins/@nocobase/plugin-acl/src/locale/en-US.json new file mode 100644 index 000000000..9435308b9 --- /dev/null +++ b/packages/plugins/@nocobase/plugin-acl/src/locale/en-US.json @@ -0,0 +1,4 @@ +{ + "The current user has no roles. Please try another account.": "The current user has no roles. Please try another account.", + "The user role does not exist. Please try signing in again": "The user role does not exist. Please try signing in again" +} diff --git a/packages/plugins/@nocobase/plugin-acl/src/locale/zh-CN.json b/packages/plugins/@nocobase/plugin-acl/src/locale/zh-CN.json new file mode 100644 index 000000000..f8f9ebc66 --- /dev/null +++ b/packages/plugins/@nocobase/plugin-acl/src/locale/zh-CN.json @@ -0,0 +1,4 @@ +{ + "The current user has no roles. Please try another account.": "当前用户没有角色，请使用其他账号。", + "The user role does not exist. Please try signing in again": "用户角色不存在，请尝试重新登录。" +} diff --git a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/setCurrentRole.test.ts b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/setCurrentRole.test.ts index cec2aaf1f..d30619694 100644 --- a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/setCurrentRole.test.ts +++ b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/setCurrentRole.test.ts @@ -23,6 +23,7 @@ describe('role', () => { state: { currentRole: '', }, + t: (key) => key, }; }); @@ -68,7 +69,10 @@ describe('role', () => { const throwFn = jest.fn(); ctx.throw = throwFn; await setCurrentRole(ctx, () => {}); - expect(throwFn).lastCalledWith(401, { code: 'ROLE_NOT_FOUND_ERR', message: 'The user role does not exist.' }); + expect(throwFn).lastCalledWith(401, { + code: 'ROLE_NOT_FOUND_ERR', + message: 'The user role does not exist. Please try signing in again', + }); expect(ctx.state.currentRole).not.toBeDefined(); }); @@ -201,7 +205,10 @@ describe('role', () => { const throwFn = jest.fn(); ctx.throw = throwFn; await setCurrentRole(ctx, () => {}); - expect(throwFn).lastCalledWith(401, { code: 'ROLE_NOT_FOUND_ERR', message: 'The user role does not exist.' }); + expect(throwFn).lastCalledWith(401, { + code: 'USER_HAS_NO_ROLES_ERR', + message: 'The current user has no roles. Please try another account.', + }); expect(ctx.state.currentRole).not.toBeDefined(); }); diff --git a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/users.test.ts b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/users.test.ts index 9db3d37d4..527fbfc1c 100644 --- a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/users.test.ts +++ b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/users.test.ts @@ -95,7 +95,7 @@ describe('actions', () => { const rolesCheckResponse2 = (await loggedAgent.set('Accept', 'application/json').get('/roles:check')) as any; expect(rolesCheckResponse2.status).toEqual(401); - expect(rolesCheckResponse2.body.errors[0].code).toEqual('ROLE_NOT_FOUND_ERR'); + expect(rolesCheckResponse2.body.errors[0].code).toEqual('USER_HAS_NO_ROLES_ERR'); }); it('should destroy through table record when destroy role', async () => { diff --git a/packages/plugins/@nocobase/plugin-acl/src/server/middlewares/setCurrentRole.ts b/packages/plugins/@nocobase/plugin-acl/src/server/middlewares/setCurrentRole.ts index 4d6f84295..94d7fc517 100644 --- a/packages/plugins/@nocobase/plugin-acl/src/server/middlewares/setCurrentRole.ts +++ b/packages/plugins/@nocobase/plugin-acl/src/server/middlewares/setCurrentRole.ts @@ -21,6 +21,13 @@ export async function setCurrentRole(ctx: Context, next) { raw: true, }), )) as Model[]; + if (!roles.length) { + ctx.state.currentRole = undefined; + return ctx.throw(401, { + code: 'USER_HAS_NO_ROLES_ERR', + message: ctx.t('The current user has no roles. Please try another account.', { ns: 'acl' }), + }); + } ctx.state.currentUser.roles = roles; // 1. If the X-Role is set, use the specified role @@ -34,7 +41,10 @@ export async function setCurrentRole(ctx: Context, next) { } if (!ctx.state.currentRole) { - return ctx.throw(401, { code: 'ROLE_NOT_FOUND_ERR', message: 'The user role does not exist.' }); + return ctx.throw(401, { + code: 'ROLE_NOT_FOUND_ERR', + message: ctx.t('The user role does not exist. Please try signing in again', { ns: 'acl' }), + }); } await next();