From 8abca1a5d9f1341d0d69cb0d9c965e56a31efaec Mon Sep 17 00:00:00 2001 From: simeng-li Date: Tue, 17 Oct 2023 17:42:04 +0800 Subject: [PATCH] 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 | 2 + packages/cli/src/connector/types.ts | 2 +- packages/core/src/__mocks__/connector.ts | 1 + packages/core/src/libraries/connector.ts | 22 +++++- .../src/routes/connector/index.post.test.ts | 15 ++++ packages/core/src/routes/connector/index.ts | 76 ++++++++++--------- 6 files changed, 78 insertions(+), 40 deletions(-) diff --git a/packages/cli/src/connector/factories.ts b/packages/cli/src/connector/factories.ts index 23e81168e..50281a726 100644 --- a/packages/cli/src/connector/factories.ts +++ b/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, }; diff --git a/packages/cli/src/connector/types.ts b/packages/cli/src/connector/types.ts index 3184a2762..2c296f545 100644 --- a/packages/cli/src/connector/types.ts +++ b/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, U extends AllConnector = AllConnector, -> = Pick & { +> = Pick & { createConnector: CreateConnector; path: string; }; diff --git a/packages/core/src/__mocks__/connector.ts b/packages/core/src/__mocks__/connector.ts index e8cdabdce..ee0a02391 100644 --- a/packages/core/src/__mocks__/connector.ts +++ b/packages/core/src/__mocks__/connector.ts @@ -59,6 +59,7 @@ export const mockConnectorFactory: ConnectorFactory = { metadata: mockMetadata, type: ConnectorType.Social, path: 'random_path', + configGuard: any(), createConnector: jest.fn(), }; diff --git a/packages/core/src/libraries/connector.ts b/packages/core/src/libraries/connector.ts index 9ec4aee01..4333ed4de 100644 --- a/packages/core/src/libraries/connector.ts +++ b/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 + ) => { + 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, }; }; diff --git a/packages/core/src/routes/connector/index.post.test.ts b/packages/core/src/routes/connector/index.post.test.ts index b049fe73b..2f16964f7 100644 --- a/packages/core/src/routes/connector/index.post.test.ts +++ b/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 + ) => { + const connectors = await getLogtoConnectors(); + + return connectors.find(({ type, metadata }) => { + return ( + type === ConnectorType.Social && + metadata.target === target && + metadata.platform === platform + ); + }); + }, }, { quota: createMockQuotaLibrary(), diff --git a/packages/core/src/routes/connector/index.ts b/packages/core/src/routes/connector/index.ts index b6c9b66a3..983494268 100644 --- a/packages/core/src/routes/connector/index.ts +++ b/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( ...[router, tenant]: RouterInitArgs ) { @@ -44,7 +46,8 @@ export default function connectorRoutes( 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( 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( 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( } 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( /** * 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(