fix(connector): passwordless connector send test msg with unsaved config (#1539)

* fix(connector): passwordless connector sendTestMsg with unsaved config

* refactor: apply suggestions from code review

Co-authored-by: Gao Sun <gao@silverhand.io>

packages/connector-aliyun-dm/src/index.ts

@@ -3,8 +3,10 @@ import {
   ConnectorErrorCodes,
   ConnectorMetadata,
   EmailSendMessageFunction,
+  EmailSendTestMessageFunction,
   EmailConnector,
   GetConnectorConfig,
+  EmailMessageTypes,
 } from '@logto/connector-types';
 import { assert } from '@silverhand/essentials';
 import { HTTPError } from 'got';
@@ -30,13 +32,27 @@ export default class AliyunDmConnector implements EmailConnector<AliyunDmConfig>
     }
   }
 
-  // eslint-disable-next-line complexity
-  public sendMessage: EmailSendMessageFunction = async (address, type, data, config) => {
-    const emailConfig = config ?? (await this.getConfig(this.metadata.id));
+  public sendMessage: EmailSendMessageFunction = async (address, type, data) => {
+    const emailConfig = await this.getConfig(this.metadata.id);
 
     this.validateConfig(emailConfig);
 
-    const { accessKeyId, accessKeySecret, accountName, fromAlias, templates } = emailConfig;
+    return this.sendMessageBy(address, type, data, emailConfig);
+  };
+
+  public sendTestMessage: EmailSendTestMessageFunction = async (config, address, type, data) => {
+    this.validateConfig(config);
+
+    return this.sendMessageBy(address, type, data, config);
+  };
+
+  private readonly sendMessageBy = async (
+    address: string,
+    type: keyof EmailMessageTypes,
+    data: EmailMessageTypes[typeof type],
+    config: AliyunDmConfig
+  ) => {
+    const { accessKeyId, accessKeySecret, accountName, fromAlias, templates } = config;
     const template = templates.find((template) => template.usageType === type);
 
     assert(

packages/connector-aliyun-sms/src/index.ts

@@ -3,8 +3,10 @@ import {
   ConnectorErrorCodes,
   ConnectorMetadata,
   SmsSendMessageFunction,
+  SmsSendTestMessageFunction,
   SmsConnector,
   GetConnectorConfig,
+  SmsMessageTypes,
 } from '@logto/connector-types';
 import { assert } from '@silverhand/essentials';
 import { HTTPError } from 'got';
@@ -25,12 +27,27 @@ export default class AliyunSmsConnector implements SmsConnector<AliyunSmsConfig>
     }
   }
 
-  public sendMessage: SmsSendMessageFunction = async (phone, type, { code }, config) => {
-    const smsConfig = config ?? (await this.getConfig(this.metadata.id));
+  public sendMessage: SmsSendMessageFunction = async (phone, type, data) => {
+    const smsConfig = await this.getConfig(this.metadata.id);
 
     this.validateConfig(smsConfig);
 
-    const { accessKeyId, accessKeySecret, signName, templates } = smsConfig;
+    return this.sendMessageBy(phone, type, data, smsConfig);
+  };
+
+  public sendTestMessage: SmsSendTestMessageFunction = async (config, phone, type, data) => {
+    this.validateConfig(config);
+
+    return this.sendMessageBy(phone, type, data, config);
+  };
+
+  private readonly sendMessageBy = async (
+    phone: string,
+    type: keyof SmsMessageTypes,
+    data: SmsMessageTypes[typeof type],
+    config: AliyunSmsConfig
+  ) => {
+    const { accessKeyId, accessKeySecret, signName, templates } = config;
     const template = templates.find(({ usageType }) => usageType === type);
 
     assert(
@@ -45,7 +62,7 @@ export default class AliyunSmsConnector implements SmsConnector<AliyunSmsConfig>
           PhoneNumbers: phone,
           SignName: signName,
           TemplateCode: template.templateCode,
-          TemplateParam: JSON.stringify({ code }),
+          TemplateParam: JSON.stringify(data),
         },
         accessKeySecret
       );

packages/connector-sendgrid-mail/src/index.ts

@@ -3,8 +3,10 @@ import {
   ConnectorErrorCodes,
   ConnectorMetadata,
   EmailSendMessageFunction,
+  EmailSendTestMessageFunction,
   EmailConnector,
   GetConnectorConfig,
+  EmailMessageTypes,
 } from '@logto/connector-types';
 import { assert } from '@silverhand/essentials';
 import got, { HTTPError } from 'got';
@@ -36,6 +38,21 @@ export default class SendGridMailConnector implements EmailConnector<SendGridMai
 
     this.validateConfig(config);
 
+    return this.sendMessageBy(address, type, data, config);
+  };
+
+  public sendTestMessage: EmailSendTestMessageFunction = async (config, address, type, data) => {
+    this.validateConfig(config);
+
+    return this.sendMessageBy(address, type, data, config);
+  };
+
+  private readonly sendMessageBy = async (
+    address: string,
+    type: keyof EmailMessageTypes,
+    data: EmailMessageTypes[typeof type],
+    config: SendGridMailConfig
+  ) => {
     const { apiKey, fromEmail, fromName, templates } = config;
     const template = templates.find((template) => template.usageType === type);
 

packages/connector-smtp/src/index.ts

@@ -3,8 +3,10 @@ import {
   ConnectorErrorCodes,
   ConnectorMetadata,
   EmailSendMessageFunction,
+  EmailSendTestMessageFunction,
   EmailConnector,
   GetConnectorConfig,
+  EmailMessageTypes,
 } from '@logto/connector-types';
 import { assert } from '@silverhand/essentials';
 import nodemailer from 'nodemailer';
@@ -31,6 +33,21 @@ export default class SmtpConnector implements EmailConnector<SmtpConfig> {
 
     this.validateConfig(config);
 
+    return this.sendMessageBy(address, type, data, config);
+  };
+
+  public sendTestMessage: EmailSendTestMessageFunction = async (config, address, type, data) => {
+    this.validateConfig(config);
+
+    return this.sendMessageBy(address, type, data, config);
+  };
+
+  private readonly sendMessageBy = async (
+    address: string,
+    type: keyof EmailMessageTypes,
+    data: EmailMessageTypes[typeof type],
+    config: SmtpConfig
+  ) => {
     const { host, port, username, password, fromEmail, replyTo, templates } = config;
     const template = templates.find((template) => template.usageType === type);
 

packages/connector-twilio-sms/src/index.ts

@@ -2,9 +2,11 @@ import {
   ConnectorError,
   ConnectorErrorCodes,
   ConnectorMetadata,
-  EmailSendMessageFunction,
+  SmsSendMessageFunction,
+  SmsSendTestMessageFunction,
   SmsConnector,
   GetConnectorConfig,
+  SmsMessageTypes,
 } from '@logto/connector-types';
 import { assert } from '@silverhand/essentials';
 import got, { HTTPError } from 'got';
@@ -25,11 +27,26 @@ export default class TwilioSmsConnector implements SmsConnector<TwilioSmsConfig>
     }
   }
 
-  public sendMessage: EmailSendMessageFunction = async (address, type, data) => {
+  public sendMessage: SmsSendMessageFunction = async (phone, type, data) => {
     const config = await this.getConfig(this.metadata.id);
 
     this.validateConfig(config);
 
+    return this.sendMessageBy(phone, type, data, config);
+  };
+
+  public sendTestMessage: SmsSendTestMessageFunction = async (config, phone, type, data) => {
+    this.validateConfig(config);
+
+    return this.sendMessageBy(phone, type, data, config);
+  };
+
+  private readonly sendMessageBy = async (
+    phone: string,
+    type: keyof SmsMessageTypes,
+    data: SmsMessageTypes[typeof type],
+    config: TwilioSmsConfig
+  ) => {
     const { accountSID, authToken, fromMessagingServiceSID, templates } = config;
     const template = templates.find((template) => template.usageType === type);
 
@@ -42,7 +59,7 @@ export default class TwilioSmsConnector implements SmsConnector<TwilioSmsConfig>
     );
 
     const parameters: PublicParameters = {
-      To: address,
+      To: phone,
       MessagingServiceSid: fromMessagingServiceSID,
       Body:
         typeof data.code === 'string'

packages/connector-types/src/index.ts

@@ -33,6 +33,7 @@ export enum ConnectorErrorCodes {
   InvalidConfig,
   InvalidResponse,
   TemplateNotFound,
+  NotImplemented,
   SocialAuthCodeInvalid,
   SocialAccessTokenInvalid,
   SocialIdTokenInvalid,
@@ -69,15 +70,27 @@ export type SmsMessageTypes = EmailMessageTypes;
 export type EmailSendMessageFunction<T = unknown> = (
   address: string,
   type: keyof EmailMessageTypes,
-  payload: EmailMessageTypes[typeof type],
-  config?: Record<string, unknown>
+  payload: EmailMessageTypes[typeof type]
+) => Promise<T>;
+
+export type EmailSendTestMessageFunction<T = unknown> = (
+  config: Record<string, unknown>,
+  address: string,
+  type: keyof EmailMessageTypes,
+  payload: EmailMessageTypes[typeof type]
 ) => Promise<T>;
 
 export type SmsSendMessageFunction<T = unknown> = (
   phone: string,
   type: keyof SmsMessageTypes,
-  payload: SmsMessageTypes[typeof type],
-  config?: Record<string, unknown>
+  payload: SmsMessageTypes[typeof type]
+) => Promise<T>;
+
+export type SmsSendTestMessageFunction<T = unknown> = (
+  config: Record<string, unknown>,
+  phone: string,
+  type: keyof SmsMessageTypes,
+  payload: SmsMessageTypes[typeof type]
 ) => Promise<T>;
 
 export interface BaseConnector<T = unknown> {
@@ -88,10 +101,12 @@ export interface BaseConnector<T = unknown> {
 
 export interface SmsConnector<T = unknown> extends BaseConnector<T> {
   sendMessage: SmsSendMessageFunction;
+  sendTestMessage?: SmsSendTestMessageFunction;
 }
 
 export interface EmailConnector<T = unknown> extends BaseConnector<T> {
   sendMessage: EmailSendMessageFunction;
+  sendTestMessage?: EmailSendTestMessageFunction;
 }
 
 export interface SocialConnector<T = unknown> extends BaseConnector<T> {

packages/core/src/middleware/koa-connector-error-handler.ts

@@ -53,6 +53,8 @@ export default function koaConnectorErrorHandler<StateT, ContextT>(): Middleware
             },
             data
           );
+        case ConnectorErrorCodes.NotImplemented:
+          throw new RequestError({ code: 'connector.not_implemented', status: 501 }, data);
         case ConnectorErrorCodes.SocialAuthCodeInvalid:
           throw new RequestError(
             {

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

@@ -1,4 +1,4 @@
-import { EmailMessageTypes, ValidateConfig } from '@logto/connector-types';
+import { ValidateConfig } from '@logto/connector-types';
 import { Connector, ConnectorType } from '@logto/schemas';
 
 import { mockConnectorInstanceList, mockMetadata, mockConnector } from '@/__mocks__';
@@ -106,7 +106,7 @@ describe('connector route', () => {
       jest.clearAllMocks();
     });
 
-    it('should get SMS connector and send message', async () => {
+    it('should get SMS connector and send test message', async () => {
       const mockedMetadata = {
         ...mockMetadata,
         type: ConnectorType.SMS,
@@ -116,59 +116,39 @@ describe('connector route', () => {
         metadata: mockedMetadata,
         validateConfig: jest.fn(),
         getConfig: jest.fn(),
-        sendMessage: async (
-          address: string,
-          type: keyof EmailMessageTypes,
-          _payload: EmailMessageTypes[typeof type]
-        ): Promise<string> => {
-          return '';
-        },
+        sendMessage: jest.fn(),
+        sendTestMessage: jest.fn(),
       };
       getConnectorInstancesPlaceHolder.mockResolvedValueOnce([mockedSmsConnectorInstance]);
-      const sendMessageSpy = jest.spyOn(mockedSmsConnectorInstance, 'sendMessage');
+      const sendMessageSpy = jest.spyOn(mockedSmsConnectorInstance, 'sendTestMessage');
       const response = await connectorRequest
         .post('/connectors/id/test')
         .send({ phone: '12345678901', config: { test: 123 } });
       expect(sendMessageSpy).toHaveBeenCalledTimes(1);
-      expect(sendMessageSpy).toHaveBeenCalledWith(
-        '12345678901',
-        'Test',
-        {
-          code: '123456',
-        },
-        { test: 123 }
-      );
+      expect(sendMessageSpy).toHaveBeenCalledWith({ test: 123 }, '12345678901', 'Test', {
+        code: '123456',
+      });
       expect(response).toHaveProperty('statusCode', 204);
     });
 
-    it('should get email connector and send message', async () => {
+    it('should get email connector and send test message', async () => {
       const mockedEmailConnector: EmailConnectorInstance = {
         connector: mockConnector,
         metadata: mockMetadata,
         validateConfig: jest.fn(),
         getConfig: jest.fn(),
-        sendMessage: async (
-          address: string,
-          type: keyof EmailMessageTypes,
-          _payload: EmailMessageTypes[typeof type]
-        ): Promise<string> => {
-          return '';
-        },
+        sendMessage: jest.fn(),
+        sendTestMessage: jest.fn(),
       };
       getConnectorInstancesPlaceHolder.mockResolvedValueOnce([mockedEmailConnector]);
-      const sendMessageSpy = jest.spyOn(mockedEmailConnector, 'sendMessage');
+      const sendMessageSpy = jest.spyOn(mockedEmailConnector, 'sendTestMessage');
       const response = await connectorRequest
         .post('/connectors/id/test')
         .send({ email: 'test@email.com', config: { test: 123 } });
       expect(sendMessageSpy).toHaveBeenCalledTimes(1);
-      expect(sendMessageSpy).toHaveBeenCalledWith(
-        'test@email.com',
-        'Test',
-        {
-          code: 'email-test',
-        },
-        { test: 123 }
-      );
+      expect(sendMessageSpy).toHaveBeenCalledWith({ test: 123 }, 'test@email.com', 'Test', {
+        code: 'email-test',
+      });
       expect(response).toHaveProperty('statusCode', 204);
     });
 

packages/core/src/routes/connector.ts

@@ -173,7 +173,7 @@ export default function connectorRoutes<T extends AuthedRouter>(router: T) {
       body: object({
         phone: string().regex(phoneRegEx).optional(),
         email: string().regex(emailRegEx).optional(),
-        config: arbitraryObjectGuard.optional(),
+        config: arbitraryObjectGuard,
       }),
     }),
     async (ctx, next) => {
@@ -205,15 +205,20 @@ export default function connectorRoutes<T extends AuthedRouter>(router: T) {
         })
       );
 
-      await connector.sendMessage(
-        subject,
-        'Test',
-        {
-          code: phone ? '123456' : 'email-test',
-        },
-        config
+      const { sendTestMessage } = connector;
+      assertThat(
+        sendTestMessage,
+        new RequestError({
+          code: 'connector.not_implemented',
+          method: 'sendTestMessage',
+          status: 501,
+        })
       );
 
+      await sendTestMessage(config, subject, 'Test', {
+        code: phone ? '123456' : 'email-test',
+      });
+
       ctx.status = 204;
 
       return next();

packages/phrases/src/languages/en/errors.ts

@@ -63,6 +63,7 @@ const errors = {
     invalid_config: "The connector's config is invalid.",
     invalid_response: "The connector's response is invalid.",
     template_not_found: 'Unable to find correct template in connector config.',
+    not_implemented: '{{method}}: has not been implemented yet.',
     invalid_access_token: "The connector's access token is invalid.",
     invalid_auth_code: "The connector's auth code is invalid.",
     invalid_id_token: "The connector's id token is invalid.",

packages/phrases/src/languages/zh-cn/errors.ts

@@ -63,6 +63,7 @@ const errors = {
     invalid_config: '连接器配置错误',
     invalid_response: '连接器错误响应',
     template_not_found: '无法从连接器配置中找到对应的模板',
+    not_implemented: '方法 {{method}} 尚未实现',
     invalid_access_token: '当前连接器的 access_token 无效',
     invalid_auth_code: '当前连接器的授权码无效',
     invalid_id_token: '当前连接器的 id_token 无效',