refactor(core): refactor post connector endpoint (#4681)

* refactor(core): refactor post connector endpoint

refactor post connector endpoint

* fix(core): fix ut

fix ut

packages/cli/src/connector/factories.ts

@@ -23,11 +23,13 @@ export const loadConnectorFactories = async (
       try {
         const createConnector = await loadConnector(packagePath, ignoreVersionMismatch);
         const rawConnector = await createConnector({ getConfig: notImplemented });
+
         validateConnectorModule(rawConnector);
 
         return {
           metadata: await parseMetadata(rawConnector.metadata, packagePath),
           type: rawConnector.type,
+          configGuard: rawConnector.configGuard,
           createConnector,
           path: packagePath,
         };

packages/cli/src/connector/types.ts

@@ -8,7 +8,7 @@ export type ConnectorFactory<
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   T extends Router<any, BaseRoutes, string>,
   U extends AllConnector = AllConnector,
-> = Pick<U, 'type' | 'metadata'> & {
+> = Pick<U, 'type' | 'metadata' | 'configGuard'> & {
   createConnector: CreateConnector<U, T>;
   path: string;
 };

packages/core/src/__mocks__/connector.ts

@@ -59,6 +59,7 @@ export const mockConnectorFactory: ConnectorFactory<typeof router> = {
   metadata: mockMetadata,
   type: ConnectorType.Social,
   path: 'random_path',
+  configGuard: any(),
   createConnector: jest.fn(),
 };
 

packages/core/src/libraries/connector.ts

@@ -1,7 +1,7 @@
 import { buildRawConnector, defaultConnectorMethods } from '@logto/cli/lib/connector/index.js';
-import type { AllConnector } from '@logto/connector-kit';
-import { validateConfig, ServiceConnector } from '@logto/connector-kit';
-import { conditional, pick, trySafe } from '@silverhand/essentials';
+import type { AllConnector, ConnectorPlatform } from '@logto/connector-kit';
+import { validateConfig, ServiceConnector, ConnectorType } from '@logto/connector-kit';
+import { type Nullable, conditional, pick, trySafe } from '@silverhand/essentials';
 
 import RequestError from '#src/errors/RequestError/index.js';
 import type Queries from '#src/tenants/Queries.js';
@@ -119,10 +119,26 @@ export const createConnectorLibrary = (
     return pickedConnector;
   };
 
+  const getLogtoConnectorByTargetAndPlatform = async (
+    target: string,
+    platform: Nullable<ConnectorPlatform>
+  ) => {
+    const connectors = await getLogtoConnectors();
+
+    return connectors.find(({ type, metadata }) => {
+      return (
+        type === ConnectorType.Social &&
+        metadata.target === target &&
+        metadata.platform === platform
+      );
+    });
+  };
+
   return {
     getConnectorConfig,
     getLogtoConnectors,
     getLogtoConnectorsWellKnown,
     getLogtoConnectorById,
+    getLogtoConnectorByTargetAndPlatform,
   };
 };

packages/core/src/routes/connector/index.post.test.ts

@@ -2,6 +2,7 @@ import { ConnectorPlatform } from '@logto/connector-kit';
 import type { Connector } from '@logto/schemas';
 import { ConnectorType } from '@logto/schemas';
 import { pickDefault, createMockUtils } from '@logto/shared/esm';
+import { type Nullable } from '@silverhand/essentials';
 import { any } from 'zod';
 
 import {
@@ -65,6 +66,20 @@ const tenantContext = new MockTenant(
 
       return connector;
     },
+    getLogtoConnectorByTargetAndPlatform: async (
+      target: string,
+      platform: Nullable<ConnectorPlatform>
+    ) => {
+      const connectors = await getLogtoConnectors();
+
+      return connectors.find(({ type, metadata }) => {
+        return (
+          type === ConnectorType.Social &&
+          metadata.target === target &&
+          metadata.platform === platform
+        );
+      });
+    },
   },
   {
     quota: createMockQuotaLibrary(),

packages/core/src/routes/connector/index.ts

@@ -1,4 +1,4 @@
-import { type ConnectorFactory, buildRawConnector } from '@logto/cli/lib/connector/index.js';
+import { type ConnectorFactory } from '@logto/cli/lib/connector/index.js';
 import type router from '@logto/cloud/routes';
 import { demoConnectorIds, validateConfig } from '@logto/connector-kit';
 import { Connectors, ConnectorType, connectorResponseGuard, type JsonObject } from '@logto/schemas';
@@ -33,6 +33,8 @@ const guardConnectorsQuota = async (
   }
 };
 
+const passwordlessConnector = new Set([ConnectorType.Email, ConnectorType.Sms]);
+
 export default function connectorRoutes<T extends AuthedRouter>(
   ...[router, tenant]: RouterInitArgs<T>
 ) {
@@ -44,7 +46,8 @@ export default function connectorRoutes<T extends AuthedRouter>(
     insertConnector,
     updateConnector,
   } = tenant.queries.connectors;
-  const { getLogtoConnectorById, getLogtoConnectors } = tenant.connectors;
+  const { getLogtoConnectorById, getLogtoConnectors, getLogtoConnectorByTargetAndPlatform } =
+    tenant.connectors;
   const {
     quota,
     signInExperiences: { removeUnavailableSocialConnectorTargets },
@@ -60,7 +63,13 @@ export default function connectorRoutes<T extends AuthedRouter>(
           metadata: true,
           syncProfile: true,
         })
-        .merge(Connectors.createGuard.pick({ id: true }).partial()), // `id` is optional
+        /* 
+          Currently the id can not be locked until the connector is successfully created.
+          Some connectors providers require a pre-generated id to complete the configuration at the IdP side.
+          Logto connector creation process currently has a hard dependency on the provider's config data.
+          A optional pre-generated id from the client side is required to complete the connector creation process.
+        */
+        .merge(Connectors.createGuard.pick({ id: true }).partial()),
       response: connectorResponseGuard,
       status: [200, 422],
     }),
@@ -84,34 +93,31 @@ export default function connectorRoutes<T extends AuthedRouter>(
 
       await guardConnectorsQuota(connectorFactory, quota);
 
-      assertThat(
-        connectorFactory.metadata.isStandard !== true || Boolean(metadata?.target),
-        'connector.should_specify_target'
-      );
-      assertThat(
-        connectorFactory.metadata.isStandard === true || metadata === undefined,
-        'connector.cannot_overwrite_metadata_for_non_standard_connector'
-      );
-
-      const { count } = await countConnectorByConnectorId(connectorId);
-      assertThat(
-        count === 0 || connectorFactory.metadata.isStandard === true,
-        new RequestError({
-          code: 'connector.multiple_instances_not_supported',
-          status: 422,
-        })
-      );
-
       if (connectorFactory.type === ConnectorType.Social) {
-        const connectors = await getLogtoConnectors();
-        const duplicateConnector = connectors
-          .filter(({ type }) => type === ConnectorType.Social)
-          .find(
-            ({ metadata: { target, platform } }) =>
-              target ===
-                (metadata ? cleanDeep(metadata).target : connectorFactory.metadata.target) &&
-              platform === connectorFactory.metadata.platform
-          );
+        assertThat(
+          connectorFactory.metadata.isStandard !== true || Boolean(metadata?.target),
+          'connector.should_specify_target'
+        );
+
+        assertThat(
+          connectorFactory.metadata.isStandard === true || metadata === undefined,
+          'connector.cannot_overwrite_metadata_for_non_standard_connector'
+        );
+
+        const { count } = await countConnectorByConnectorId(connectorId);
+        assertThat(
+          count === 0 || connectorFactory.metadata.isStandard === true,
+          new RequestError({
+            code: 'connector.multiple_instances_not_supported',
+            status: 422,
+          })
+        );
+
+        const duplicateConnector = await getLogtoConnectorByTargetAndPlatform(
+          // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
+          metadata?.target || connectorFactory.metadata.target,
+          connectorFactory.metadata.platform
+        );
         assertThat(
           !duplicateConnector,
           new RequestError(
@@ -128,11 +134,11 @@ export default function connectorRoutes<T extends AuthedRouter>(
       }
 
       if (config) {
-        const { rawConnector } = await buildRawConnector(connectorFactory);
-        validateConfig(config, rawConnector.configGuard);
+        validateConfig(config, connectorFactory.configGuard);
       }
 
       const insertConnectorId = proposedId ?? generateStandardShortId();
+
       await insertConnector({
         id: insertConnectorId,
         connectorId,
@@ -142,11 +148,9 @@ export default function connectorRoutes<T extends AuthedRouter>(
       /**
        * We can have only one working email/sms connector:
        * once we insert a new one, old connectors with same type should be deleted.
+       * TODO: should using transaction to ensure the atomicity of the operation. LOG-7260
        */
-      if (
-        connectorFactory.type === ConnectorType.Sms ||
-        connectorFactory.type === ConnectorType.Email
-      ) {
+      if (passwordlessConnector.has(connectorFactory.type)) {
         const logtoConnectors = await getLogtoConnectors();
         const conflictingConnectorIds = logtoConnectors
           .filter(