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 <chenlinxh@gmail.com>
2023-12-15 21:03:16 +08:00 · 2023-12-15 21:03:16 +08:00 · cb6a6b87c9
commit cb6a6b87c9
parent 7d2fe69944
17 changed files with 117 additions and 37 deletions
--- 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,
      };
--- 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 });
    }
  }

--- 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 = {};
--- 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<AppComponentProps> = 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<AppComponentProps> = observer((props) => {
  if (!app.maintained && app.maintaining) return app.renderComponent('AppMaintaining', { app });
  if (app.error?.code === 'LOAD_ERROR') return <AppError app={app} error={app.error} />;
  return (
-    <ErrorBoundary FallbackComponent={(props) => <AppError {...props} app={app} />} onError={handleErrors}>
+    <ErrorBoundary
+      FallbackComponent={(props) => <AppError app={app} error={app.error} {...props} />}
+      onError={handleErrors}
+    >
      <ApplicationContext.Provider value={app}>
        {app.maintained && app.maintaining && app.renderComponent('AppMaintainingDialog', { app })}
        {app.renderComponent('AppMain')}
--- 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 }) => (
  </div>
 );

-const AppNotFound: FC = () => <div>Not Found</div>;
+const AppNotFound: FC = () => <div></div>;

 export const defaultAppComponents = {
  AppMain: MainComponent,
--- 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 <div style={{ color: '#ccc' }}>{t('Oops! No authentication methods available.')}</div>;
  }
--- 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</1>": "This is likely a NocoBase internals bug. Please open an issue at <1>here</1>",
  "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"
 }
--- 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": "没有权限"
 }
--- 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: [
+          <Button
+            type="primary"
+            key="try"
+            onClick={() => {
+              api.auth.setToken(null);
+              window.location.reload();
+            }}
+          >
+            {app.i18n.t('Sign in with another account')}
+          </Button>,
+          subApp ? (
+            <Button key="back" onClick={() => (window.location.href = '/admin')}>
+              {app.i18n.t('Return to the main application')}
+            </Button>
+          ) : null,
+        ],
+      };
+    default:
+      return {};
+  }
+};
+
 const AppError: FC<{ error: Error; app: Application }> = observer(({ app, error }) => {
+  const props = useErrorProps(app, error);
  return (
    <div>
      <Result
@ -43,13 +81,14 @@ const AppError: FC<{ error: Error; app: Application }> = 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={[
          <Button type="primary" key="try" onClick={() => window.location.reload()}>
            {app.i18n.t('Try again')}
          </Button>,
        ]}
+        {...props}
      />
    </div>
  );
--- 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);
      }
--- a/packages/core/database/src/repository.ts
+++ b/packages/core/database/src/repository.ts
@ -479,7 +479,6 @@ export class Repository<TModelAttributes extends {} = any, TCreationAttributes e
    const instance = await this.findOne({ filter, transaction });

    if (instance) {
-      console.log(filter, instance.toJSON(), instance.get(this.collection.model.primaryKeyAttribute));
      return await this.update({
        filterByTk: instance.get(this.collection.filterTargetKey || this.collection.model.primaryKeyAttribute),
        values,
--- a/packages/core/server/src/locale/locale.ts
+++ b/packages/core/server/src/locale/locale.ts
@ -96,6 +96,7 @@ export class Locale {
    Object.keys(resources).forEach((name) => {
      this.app.i18n.addResources(lang, name, resources[name]);
    });
+
    return resources;
  }

--- a/packages/plugins/@nocobase/plugin-acl/src/locale/en-US.json
+++ 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"
+}
--- a/packages/plugins/@nocobase/plugin-acl/src/locale/zh-CN.json
+++ 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": "用户角色不存在，请尝试重新登录。"
+}
--- 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();
  });

--- 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 () => {
--- 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();