fix: target can not be updated (#2609)

2025-03-31 22:51:25 -05:00 · 2022-12-13 10:19:42 +08:00 · 2022-12-13 10:19:42 +08:00 · 14f86c01d5
commit 14f86c01d5
parent 3db51178a4
13 changed files with 70 additions and 39 deletions
--- a/packages/core/src/routes/connector.test.ts
+++ b/packages/core/src/routes/connector.test.ts
@ -188,7 +188,6 @@ describe('connector route', () => {
          metadata: { ...mockConnectorFactory.metadata, id: 'connectorId' },
        },
      ]);
-      countConnectorByConnectorId.mockResolvedValueOnce({ count: 0 });
      const response = await connectorRequest.post('/connectors').send({
        connectorId: 'id0',
        config: { cliend_id: 'client_id', client_secret: 'client_secret' },
@ -255,7 +254,7 @@ describe('connector route', () => {
        {
          ...mockConnectorFactory,
          type: ConnectorType.Sms,
-          metadata: { ...mockConnectorFactory.metadata, id: 'id0', isStandard: true },
+          metadata: { ...mockConnectorFactory.metadata, id: 'id1' },
        },
      ]);
      getLogtoConnectors.mockResolvedValueOnce([
@ -266,20 +265,19 @@ describe('connector route', () => {
          ...mockLogtoConnector,
        },
      ]);
+      countConnectorByConnectorId.mockResolvedValueOnce({ count: 0 });
      const response = await connectorRequest.post('/connectors').send({
-        connectorId: 'id0',
+        connectorId: 'id1',
        config: { cliend_id: 'client_id', client_secret: 'client_secret' },
-        metadata: { target: 'target', name: { en: '' }, logo: '', logoDark: null },
      });
      expect(response).toHaveProperty('statusCode', 200);
      expect(response.body).toMatchObject(
        expect.objectContaining({
-          connectorId: 'id0',
+          connectorId: 'id1',
          config: {
            cliend_id: 'client_id',
            client_secret: 'client_secret',
          },
-          metadata: { target: 'target' },
        })
      );
      expect(deleteConnectorByIds).toHaveBeenCalledWith(['id']);
@ -347,6 +345,7 @@ describe('connector route', () => {
      ]);
      const response = await connectorRequest.post('/connectors').send({
        connectorId: 'id0',
+        metadata: { target: 'target' },
      });
      expect(response).toHaveProperty('statusCode', 422);
    });
--- a/packages/core/src/routes/connector.ts
+++ b/packages/core/src/routes/connector.ts
@ -3,7 +3,6 @@ import { emailRegEx, phoneRegEx } from '@logto/core-kit';
 import type { ConnectorFactoryResponse, ConnectorResponse } from '@logto/schemas';
 import { arbitraryObjectGuard, Connectors, ConnectorType } from '@logto/schemas';
 import { buildIdGenerator } from '@logto/shared';
-import { conditional } from '@silverhand/essentials';
 import cleanDeep from 'clean-deep';
 import { object, string } from 'zod';

@ -114,9 +113,9 @@ export default function connectorRoutes<T extends AuthedRouter>(router: T) {
    // eslint-disable-next-line complexity
    async (ctx, next) => {
      const {
-        body: { connectorId },
-        body,
+        body: { connectorId, metadata, config, syncProfile },
      } = ctx.guard;
+
      const connectorFactories = await loadConnectorFactories();
      const connectorFactory = connectorFactories.find(
        ({ metadata: { id } }) => id === connectorId
@ -129,6 +128,15 @@ export default function connectorRoutes<T extends AuthedRouter>(router: T) {
        });
      }

+      assertThat(
+        connectorFactory.metadata.isStandard !== true || metadata?.target,
+        'connector.can_not_modify_target'
+      );
+      assertThat(
+        connectorFactory.metadata.isStandard === true || metadata === undefined,
+        'connector.cannot_change_metadata_for_non_standard_connector'
+      );
+
      const { count } = await countConnectorByConnectorId(connectorId);
      assertThat(
        count === 0 || connectorFactory.metadata.isStandard === true,
@ -140,25 +148,23 @@ export default function connectorRoutes<T extends AuthedRouter>(router: T) {

      if (connectorFactory.type === ConnectorType.Social) {
        const connectors = await getLogtoConnectors();
-        const connectorTarget = body.metadata?.target ?? connectorFactory.metadata.target;
        assertThat(
          !connectors
            .filter(({ type }) => type === ConnectorType.Social)
            .some(
              ({ metadata: { target, platform } }) =>
-                target === connectorTarget && platform === connectorFactory.metadata.platform
+                target === cleanDeep(metadata)?.target &&
+                platform === connectorFactory.metadata.platform
            ),
          new RequestError({ code: 'connector.multiple_target_with_same_platform', status: 422 })
        );
      }

      const insertConnectorId = generateConnectorId();
-      const { metadata, ...rest } = body;
-
      ctx.body = await insertConnector({
        id: insertConnectorId,
-        ...conditional(metadata && { metadata: cleanDeep(metadata) }),
-        ...rest,
+        connectorId,
+        ...cleanDeep({ syncProfile, config, metadata }),
      });

      /**
@ -194,17 +200,25 @@ export default function connectorRoutes<T extends AuthedRouter>(router: T) {
        .pick({ config: true, metadata: true, syncProfile: true })
        .partial(),
    }),
-
    async (ctx, next) => {
      const {
        params: { id },
-        body: { config },
-        body,
+        body: { config, metadata, syncProfile },
      } = ctx.guard;

-      const { type, validateConfig } = await getLogtoConnectorById(id);
+      const { type, validateConfig, metadata: originalMetadata } = await getLogtoConnectorById(id);

-      if (body.syncProfile) {
+      assertThat(
+        originalMetadata.isStandard !== true || metadata?.target === originalMetadata.target,
+        'connector.can_not_modify_target'
+      );
+
+      assertThat(
+        originalMetadata.isStandard === true || metadata === undefined,
+        'connector.cannot_change_metadata_for_non_standard_connector'
+      );
+
+      if (syncProfile) {
        assertThat(
          type === ConnectorType.Social,
          new RequestError({ code: 'connector.invalid_type_for_syncing_profile', status: 422 })
@ -214,12 +228,9 @@ export default function connectorRoutes<T extends AuthedRouter>(router: T) {
      if (config) {
        validateConfig(config);
      }
-      // Once created, target can not be modified.
-      assertThat(body.metadata?.target === undefined, 'connector.can_not_modify_target');

-      const { metadata: databaseMetadata, ...rest } = body;
      await updateConnector({
-        set: databaseMetadata ? { metadata: cleanDeep(databaseMetadata), ...rest } : rest,
+        set: cleanDeep({ config, metadata, syncProfile }),
        where: { id },
        jsonbMode: 'replace',
      });
--- a/packages/core/src/routes/connector.update.test.ts
+++ b/packages/core/src/routes/connector.update.test.ts
@ -120,7 +120,7 @@ describe('connector PATCH routes', () => {
      updateConnector.mockResolvedValueOnce({
        ...mockConnector,
        metadata: {
-          target: 'target',
+          target: 'connector',
          name: { en: 'connector_name', fr: 'connector_name' },
          logo: 'new_logo.png',
        },
@ -131,6 +131,7 @@ describe('connector PATCH routes', () => {
          name: { en: 'connector_name', fr: 'connector_name' },
          logo: 'new_logo.png',
          logoDark: null,
+          target: 'connector',
        },
      });
      expect(updateConnector).toHaveBeenCalledWith(
@ -141,6 +142,7 @@ describe('connector PATCH routes', () => {
            metadata: {
              name: { en: 'connector_name', fr: 'connector_name' },
              logo: 'new_logo.png',
+              target: 'connector',
            },
          },
          jsonbMode: 'replace',
@ -161,24 +163,20 @@ describe('connector PATCH routes', () => {
      updateConnector.mockResolvedValueOnce({
        ...mockConnector,
        metadata: {
-          target: '',
+          target: 'connector',
          name: { en: '' },
          logo: '',
          logoDark: '',
        },
      });
      const response = await connectorRequest.patch('/connectors/id').send({
-        metadata: {
-          name: { en: '' },
-          logo: '',
-          logoDark: '',
-        },
+        metadata: { target: 'connector', name: { en: '' }, logo: '', logoDark: '' },
      });
      expect(updateConnector).toHaveBeenCalledWith(
        expect.objectContaining({
          where: { id: 'id' },
          set: {
-            metadata: {},
+            metadata: { target: 'connector' },
          },
          jsonbMode: 'replace',
        })
--- a/packages/integration-tests/src/api/connector.ts
+++ b/packages/integration-tests/src/api/connector.ts
@ -9,22 +9,26 @@ export const getConnector = async (connectorId: string) =>
  authedAdminApi.get(`connectors/${connectorId}`).json<ConnectorResponse>();

 // FIXME @Darcy: correct use of `id` and `connectorId`.
-export const postConnector = async (connectorId: string) =>
+export const postConnector = async (connectorId: string, metadata?: Record<string, unknown>) =>
  authedAdminApi
    .post({
      url: `connectors`,
-      json: { connectorId },
+      json: { connectorId, metadata },
    })
    .json<Connector>();

 export const deleteConnectorById = async (id: string) =>
  authedAdminApi.delete({ url: `connectors/${id}` }).json();

-export const updateConnectorConfig = async (connectorId: string, config: Record<string, unknown>) =>
+export const updateConnectorConfig = async (
+  connectorId: string,
+  config: Record<string, unknown>,
+  metadata?: Record<string, unknown>
+) =>
  authedAdminApi
    .patch({
      url: `connectors/${connectorId}`,
-      json: { config },
+      json: { config, metadata },
    })
    .json<ConnectorResponse>();

--- a/packages/integration-tests/tests/api/connector.test.ts
+++ b/packages/integration-tests/tests/api/connector.test.ts
@ -71,13 +71,18 @@ test('connector set-up flow', async () => {
  /*
   * Change to another SMS/Email connector
   */
-  const { id } = await postConnector(mockStandardEmailConnectorId);
-  await updateConnectorConfig(id, mockStandardEmailConnectorConfig);
+  const { id } = await postConnector(mockStandardEmailConnectorId, {
+    target: 'mock-standard-mail',
+  }); // TODO [LOG-4862]: update mock connector
+  await updateConnectorConfig(id, mockStandardEmailConnectorConfig, {
+    target: 'mock-standard-mail',
+  }); // TODO [LOG-4862]: update mock connector
  connectorIdMap.set(mockStandardEmailConnectorId, id);
  const currentConnectors = await listConnectors();
  expect(
    currentConnectors.some((connector) => connector.connectorId === mockEmailConnectorId)
  ).toBeFalsy();
+  connectorIdMap.delete(mockEmailConnectorId);
  expect(
    currentConnectors.some((connector) => connector.connectorId === mockStandardEmailConnectorId)
  ).toBeTruthy();
@ -85,7 +90,6 @@ test('connector set-up flow', async () => {
    currentConnectors.find((connector) => connector.connectorId === mockStandardEmailConnectorId)
      ?.config
  ).toEqual(mockStandardEmailConnectorConfig);
-  connectorIdMap.delete(mockEmailConnectorId);

  /*
   * Delete (i.e. disable) a connector
--- a/packages/phrases/src/locales/de/errors.ts
+++ b/packages/phrases/src/locales/de/errors.ts
@ -112,6 +112,8 @@ const errors = {
    can_not_modify_target: 'The connector target can not be modified.',
    multiple_target_with_same_platform:
      'You can not have multiple social connectors that have same target and platform.',
+    cannot_change_metadata_for_non_standard_connector:
+      "This connector's `metadata` cannot be changed.",
  },
  passcode: {
    phone_email_empty: 'Telefonnummer oder E-Mail darf nicht leer sein.',
--- a/packages/phrases/src/locales/en/errors.ts
+++ b/packages/phrases/src/locales/en/errors.ts
@ -111,6 +111,8 @@ const errors = {
    can_not_modify_target: 'The connector target can not be modified.',
    multiple_target_with_same_platform:
      'You can not have multiple social connectors that have same target and platform.',
+    cannot_change_metadata_for_non_standard_connector:
+      "This connector's `metadata` cannot be changed.",
  },
  passcode: {
    phone_email_empty: 'Both phone and email are empty.',
--- a/packages/phrases/src/locales/fr/errors.ts
+++ b/packages/phrases/src/locales/fr/errors.ts
@ -118,6 +118,8 @@ const errors = {
    can_not_modify_target: 'The connector target can not be modified.', // UNTRANSLATED
    multiple_target_with_same_platform:
      'You can not have multiple social connectors that have same target and platform.', // UNTRANSLATED
+    cannot_change_metadata_for_non_standard_connector:
+      "This connector's `metadata` cannot be changed.", // UNTRANSLATED
  },
  passcode: {
    phone_email_empty: "Le téléphone et l'email sont vides.",
--- a/packages/phrases/src/locales/ko/errors.ts
+++ b/packages/phrases/src/locales/ko/errors.ts
@ -110,6 +110,8 @@ const errors = {
    can_not_modify_target: 'The connector target can not be modified.', // UNTRANSLATED
    multiple_target_with_same_platform:
      'You can not have multiple social connectors that have same target and platform.', // UNTRANSLATED
+    cannot_change_metadata_for_non_standard_connector:
+      "This connector's `metadata` cannot be changed.", // UNTRANSLATED
  },
  passcode: {
    phone_email_empty: '휴대전화번호 그리고 이메일이 비어있어요.',
--- a/packages/phrases/src/locales/pt-br/errors.ts
+++ b/packages/phrases/src/locales/pt-br/errors.ts
@ -115,6 +115,8 @@ const errors = {
    can_not_modify_target: 'O destino do conector não pode ser modificado.',
    multiple_target_with_same_platform:
      'Você não pode ter vários conectores sociais com o mesmo destino e plataforma.',
+    cannot_change_metadata_for_non_standard_connector:
+      "This connector's `metadata` cannot be changed.", // UNTRANSLATED
  },
  passcode: {
    phone_email_empty: 'Telefone e e-mail estão vazios.',
--- a/packages/phrases/src/locales/pt-pt/errors.ts
+++ b/packages/phrases/src/locales/pt-pt/errors.ts
@ -113,6 +113,8 @@ const errors = {
    can_not_modify_target: 'The connector target can not be modified.', // UNTRANSLATED
    multiple_target_with_same_platform:
      'You can not have multiple social connectors that have same target and platform.', // UNTRANSLATED
+    cannot_change_metadata_for_non_standard_connector:
+      "This connector's `metadata` cannot be changed.", // UNTRANSLATED
  },
  passcode: {
    phone_email_empty: 'O campos telefone e email estão vazios.',
--- a/packages/phrases/src/locales/tr-tr/errors.ts
+++ b/packages/phrases/src/locales/tr-tr/errors.ts
@ -112,6 +112,8 @@ const errors = {
    can_not_modify_target: 'The connector target can not be modified.', // UNTRANSLATED
    multiple_target_with_same_platform:
      'You can not have multiple social connectors that have same target and platform.', // UNTRANSLATED
+    cannot_change_metadata_for_non_standard_connector:
+      "This connector's `metadata` cannot be changed.", // UNTRANSLATED
  },
  passcode: {
    phone_email_empty: 'Hem telefon hem de e-posta adresi yok.',
--- a/packages/phrases/src/locales/zh-cn/errors.ts
+++ b/packages/phrases/src/locales/zh-cn/errors.ts
@ -103,6 +103,7 @@ const errors = {
    invalid_type_for_syncing_profile: '只有社交连接器可以开启用户档案同步。',
    can_not_modify_target: '不可修改连接器 target。',
    multiple_target_with_same_platform: '不能同时存在多个有相同 target 和平台类型的社交连接器。',
+    cannot_change_metadata_for_non_standard_connector: '不可配置该连接器的 metadata 参数。',
  },
  passcode: {
    phone_email_empty: '手机号与邮箱地址均为空',