feat(core)!: use comma separated values as a string array in the env file (#1762)

packages/core/jest.setup.ts

@@ -6,10 +6,11 @@ import envSet from '@/env-set';
 
 import { privateKeyPath } from './jest.global-setup';
 
+// eslint-disable-next-line unicorn/prefer-top-level-await
 (async () => {
   process.env = {
     ...process.env,
-    OIDC_PRIVATE_KEY_PATHS: `["${privateKeyPath}"]`,
+    OIDC_PRIVATE_KEY_PATHS: privateKeyPath,
     OIDC_COOKIE_KEYS: '["LOGTOSEKRIT1"]',
   };
   await envSet.load();

packages/core/src/connectors/consts.ts

@@ -1,6 +1,4 @@
-import { getEnv } from '@silverhand/essentials';
-
-const defaultPackages = [
+export const defaultConnectorPackages = [
   '@logto/connector-alipay-web',
   '@logto/connector-alipay-native',
   '@logto/connector-aliyun-dm',
@@ -16,9 +14,3 @@ const defaultPackages = [
   '@logto/connector-wechat-web',
   '@logto/connector-wechat-native',
 ];
-
-const additionalConnectorPackages = getEnv('ADDITIONAL_CONNECTOR_PACKAGES', '')
-  .split(',')
-  .filter(Boolean);
-
-export const connectorPackages = [...defaultPackages, ...additionalConnectorPackages];

packages/core/src/connectors/index.ts

@@ -4,10 +4,11 @@ import path from 'path';
 import { ConnectorInstance, SocialConnectorInstance } from '@logto/connector-types';
 import resolvePackagePath from 'resolve-package-path';
 
+import envSet from '@/env-set';
 import RequestError from '@/errors/RequestError';
 import { findAllConnectors, insertConnector } from '@/queries/connector';
 
-import { connectorPackages } from './consts';
+import { defaultConnectorPackages } from './consts';
 import { ConnectorType } from './types';
 import { getConnectorConfig } from './utilities';
 
@@ -19,6 +20,12 @@ const loadConnectors = async () => {
     return cachedConnectors;
   }
 
+  const {
+    values: { additionalConnectorPackages },
+  } = envSet;
+
+  const connectorPackages = [...defaultConnectorPackages, ...additionalConnectorPackages];
+
   // eslint-disable-next-line @silverhand/fp/no-mutation
   cachedConnectors = await Promise.all(
     connectorPackages.map(async (packageName) => {

packages/core/src/env-set/index.ts

@@ -6,6 +6,7 @@ import { appendPath } from '@/utils/url';
 import createPoolByEnv from './create-pool-by-env';
 import loadOidcValues from './oidc';
 import { isTrue } from './parameters';
+import { getEnvAsStringArray } from './utils';
 
 export enum MountedApps {
   Api = 'api',
@@ -23,6 +24,7 @@ const loadEnvValues = async () => {
   const port = Number(getEnv('PORT', '3001'));
   const localhostUrl = `${isHttpsEnabled ? 'https' : 'http'}://localhost:${port}`;
   const endpoint = getEnv('ENDPOINT', localhostUrl);
+  const additionalConnectorPackages = getEnvAsStringArray('ADDITIONAL_CONNECTOR_PACKAGES', []);
 
   return Object.freeze({
     isTest,
@@ -34,6 +36,7 @@ const loadEnvValues = async () => {
     port,
     localhostUrl,
     endpoint,
+    additionalConnectorPackages,
     developmentUserId: getEnv('DEVELOPMENT_USER_ID'),
     trustProxyHeader: isTrue(getEnv('TRUST_PROXY_HEADER')),
     oidc: await loadOidcValues(appendPath(endpoint, '/oidc').toString()),

packages/core/src/env-set/oidc.test.ts

@@ -3,7 +3,7 @@ import fs, { PathOrFileDescriptor } from 'fs';
 
 import inquirer from 'inquirer';
 
-import { readPrivateKeys } from './oidc';
+import { readCookieKeys, readPrivateKeys } from './oidc';
 
 describe('oidc env-set', () => {
   const envBackup = process.env;
@@ -17,24 +17,24 @@ describe('oidc env-set', () => {
     jest.resetModules();
   });
 
-  it('should read private keys if `OIDC_PRIVATE_KEYS` is provided', async () => {
-    process.env.OIDC_PRIVATE_KEYS = '["foo","bar"]';
+  it('should read OIDC private keys if `OIDC_PRIVATE_KEYS` is provided', async () => {
+    process.env.OIDC_PRIVATE_KEYS = 'foo, bar';
 
     const privateKeys = await readPrivateKeys();
 
     expect(privateKeys).toEqual(['foo', 'bar']);
   });
 
-  it('should read private keys if `OIDC_PRIVATE_KEY` is provided - [compatibility]', async () => {
-    process.env.OIDC_PRIVATE_KEY = 'foo';
+  it('should read OIDC private keys if provided `OIDC_PRIVATE_KEYS` contain newline characters', async () => {
+    process.env.OIDC_PRIVATE_KEYS = 'foo\nbar, bob\noop';
 
     const privateKeys = await readPrivateKeys();
 
-    expect(privateKeys).toEqual(['foo']);
+    expect(privateKeys).toEqual(['foo\nbar', 'bob\noop']);
   });
 
-  it('should read private keys if `OIDC_PRIVATE_KEY_PATHS` is provided', async () => {
-    process.env.OIDC_PRIVATE_KEY_PATHS = '["foo.pem", "bar.pem"]';
+  it('should read OIDC private keys if `OIDC_PRIVATE_KEY_PATHS` is provided', async () => {
+    process.env.OIDC_PRIVATE_KEY_PATHS = 'foo.pem, bar.pem';
     const existsSyncSpy = jest.spyOn(fs, 'existsSync').mockReturnValue(true);
     const readFileSyncSpy = jest
       .spyOn(fs, 'readFileSync')
@@ -48,45 +48,12 @@ describe('oidc env-set', () => {
     readFileSyncSpy.mockRestore();
   });
 
-  it('should read private keys if `OIDC_PRIVATE_KEY_PATH` is provided - [compatibility]', async () => {
-    // Unset the `OIDC_PRIVATE_KEY_PATHS` environment variable config by jest.setup.ts.
-    process.env.OIDC_PRIVATE_KEY_PATHS = '';
-
-    process.env.OIDC_PRIVATE_KEY_PATH = 'foo.pem';
-
-    const existsSyncSpy = jest.spyOn(fs, 'existsSync').mockReturnValue(true);
-
-    const readFileSyncSpy = jest
-      .spyOn(fs, 'readFileSync')
-      .mockImplementation((path: PathOrFileDescriptor) => path.toString());
-
-    const privateKeys = await readPrivateKeys();
-
-    expect(privateKeys).toEqual(['foo.pem']);
-
-    existsSyncSpy.mockRestore();
-    readFileSyncSpy.mockRestore();
-  });
-
-  it('should throw if private keys configured in `OIDC_PRIVATE_KEY_PATHS` are not found', async () => {
-    process.env.OIDC_PRIVATE_KEY_PATHS = '["foo.pem","bar.pem"]';
-    const existsSyncSpy = jest.spyOn(fs, 'existsSync').mockReturnValue(false);
-
-    await expect(readPrivateKeys).rejects.toMatchError(
-      new Error(
-        `Private keys ${process.env.OIDC_PRIVATE_KEY_PATHS} configured in env \`OIDC_PRIVATE_KEY_PATHS\` not found.`
-      )
-    );
-
-    existsSyncSpy.mockRestore();
-  });
-
-  it('should generate a default private key if `OIDC_PRIVATE_KEY_PATHS` and `OIDC_PRIVATE_KEYS` are not provided', async () => {
+  it('should generate a default OIDC private key if neither `OIDC_PRIVATE_KEY_PATHS` nor `OIDC_PRIVATE_KEYS` is provided', async () => {
     process.env.OIDC_PRIVATE_KEYS = '';
     process.env.OIDC_PRIVATE_KEY_PATHS = '';
 
     const readFileSyncSpy = jest.spyOn(fs, 'readFileSync').mockImplementation(() => {
-      throw new Error('Intent read file error');
+      throw new Error('Dummy read file error');
     });
 
     // eslint-disable-next-line @typescript-eslint/no-empty-function
@@ -107,4 +74,42 @@ describe('oidc env-set', () => {
     generateKeyPairSyncSpy.mockRestore();
     writeFileSyncSpy.mockRestore();
   });
+
+  it('should throw if private keys configured in `OIDC_PRIVATE_KEY_PATHS` are not found', async () => {
+    process.env.OIDC_PRIVATE_KEY_PATHS = 'foo.pem, bar.pem, baz.pem';
+    const existsSyncSpy = jest.spyOn(fs, 'existsSync').mockReturnValue(false);
+
+    await expect(readPrivateKeys()).rejects.toMatchError(
+      new Error(
+        `Private keys foo.pem, bar.pem, and baz.pem configured in env \`OIDC_PRIVATE_KEY_PATHS\` not found.`
+      )
+    );
+
+    existsSyncSpy.mockRestore();
+  });
+
+  it('should read OIDC cookie keys if `OIDC_COOKIE_KEYS` is provided', async () => {
+    process.env.OIDC_COOKIE_KEYS = 'foo, bar';
+
+    const cookieKeys = await readCookieKeys();
+
+    expect(cookieKeys).toEqual(['foo', 'bar']);
+  });
+
+  it('should generate a default OIDC cookie key if `OIDC_COOKIE_KEYS` is not provided', async () => {
+    process.env.OIDC_COOKIE_KEYS = '';
+
+    const promptMock = jest.spyOn(inquirer, 'prompt').mockResolvedValue({ confirm: true });
+
+    // eslint-disable-next-line @typescript-eslint/no-empty-function
+    const appendFileSyncSpy = jest.spyOn(fs, 'appendFileSync').mockImplementation(() => {});
+
+    const cookieKeys = await readCookieKeys();
+
+    expect(cookieKeys.length).toBe(1);
+    expect(appendFileSyncSpy).toHaveBeenCalled();
+
+    promptMock.mockRestore();
+    appendFileSyncSpy.mockRestore();
+  });
 });

packages/core/src/env-set/oidc.ts

@@ -14,6 +14,8 @@ import { getEnvAsStringArray } from './utils';
 
 const defaultLogtoOidcPrivateKey = './oidc-private-key.pem';
 
+const listFormatter = new Intl.ListFormat('en', { style: 'long', type: 'conjunction' });
+
 /**
  * Try to read private keys with the following order:
  *
@@ -32,19 +34,7 @@ export const readPrivateKeys = async (): Promise<string[]> => {
     return privateKeys;
   }
 
-  // Downward compatibility for `OIDC_PRIVATE_KEY`
-  const compatPrivateKey = getEnv('OIDC_PRIVATE_KEY');
-
-  if (compatPrivateKey) {
-    return [compatPrivateKey];
-  }
-
-  // Downward compatibility for `OIDC_PRIVATE_KEY_PATH`
-  const originPrivateKeyPath = getEnv('OIDC_PRIVATE_KEY_PATH');
-  const privateKeyPaths = getEnvAsStringArray(
-    'OIDC_PRIVATE_KEY_PATHS',
-    originPrivateKeyPath ? [originPrivateKeyPath] : []
-  );
+  const privateKeyPaths = getEnvAsStringArray('OIDC_PRIVATE_KEY_PATHS');
 
   // If no private key path is found, ask the user to generate a new one.
   if (privateKeyPaths.length === 0) {
@@ -87,18 +77,20 @@ export const readPrivateKeys = async (): Promise<string[]> => {
   const notExistPrivateKeys = privateKeyPaths.filter((path): boolean => !existsSync(path));
 
   if (notExistPrivateKeys.length > 0) {
-    const notExistPrivateKeysRawValue = JSON.stringify(notExistPrivateKeys);
     throw new Error(
-      `Private keys ${notExistPrivateKeysRawValue} configured in env \`OIDC_PRIVATE_KEY_PATHS\` not found.`
+      `Private keys ${listFormatter.format(
+        notExistPrivateKeys
+      )} configured in env \`OIDC_PRIVATE_KEY_PATHS\` not found.`
     );
   }
 
   try {
     return privateKeyPaths.map((path): string => readFileSync(path, 'utf8'));
   } catch {
-    const privateKeyPathsRawValue = JSON.stringify(privateKeyPaths);
     throw new Error(
-      `Failed to read private keys from ${privateKeyPathsRawValue} in env \`OIDC_PRIVATE_KEY_PATHS\`.`
+      `Failed to read private keys ${listFormatter.format(
+        privateKeyPaths
+      )} from env \`OIDC_PRIVATE_KEY_PATHS\`.`
     );
   }
 };
@@ -110,7 +102,7 @@ export const readPrivateKeys = async (): Promise<string[]> => {
  *
  * @returns The cookie keys in array.
  */
-const readCookieKeys = async (): Promise<string[]> => {
+export const readCookieKeys = async (): Promise<string[]> => {
   const envKey = 'OIDC_COOKIE_KEYS';
   const keys = getEnvAsStringArray(envKey);
 
@@ -119,7 +111,7 @@ const readCookieKeys = async (): Promise<string[]> => {
   }
 
   const cookieKeysMissingError = new Error(
-    `The OIDC cookie keys array is missing, Please check the value of env \`${envKey}\`.`
+    `The OIDC cookie keys are not found. Please check the value of env \`${envKey}\`.`
   );
 
   if (noInquiry) {
@@ -138,10 +130,10 @@ const readCookieKeys = async (): Promise<string[]> => {
     }
   }
 
-  const generated = [nanoid()];
-  appendDotEnv(envKey, JSON.stringify(generated));
+  const generated = nanoid();
+  appendDotEnv(envKey, generated);
 
-  return generated;
+  return [generated];
 };
 
 const loadOidcValues = async (issuer: string) => {

packages/core/src/env-set/utils.ts

@@ -1,16 +1,5 @@
 import { getEnv } from '@silverhand/essentials';
 
-class EnvParseError extends Error {
-  constructor(envKey: string, errorMessage: string) {
-    super(`Failed to parse env \`${envKey}\`: ${errorMessage}`);
-  }
-}
-
-class InvalidStringArrayValueError extends Error {
-  message = 'the value should be an array of strings.';
-}
-
-// TODO: LOG-3870 - Add `getEnvAsStringArray` to `@silverhand/essentials`
 export const getEnvAsStringArray = (envKey: string, fallback: string[] = []): string[] => {
   const rawValue = getEnv(envKey);
 
@@ -18,33 +7,8 @@ export const getEnvAsStringArray = (envKey: string, fallback: string[] = []): st
     return fallback;
   }
 
-  try {
-    return convertEnvToStringArray(rawValue);
-  } catch (error: unknown) {
-    if (error instanceof InvalidStringArrayValueError) {
-      throw new EnvParseError(envKey, error.message);
-    }
-    throw error;
-  }
-};
-
-const convertEnvToStringArray = (value: string): string[] => {
-  // eslint-disable-next-line @silverhand/fp/no-let
-  let values: unknown;
-
-  try {
-    // eslint-disable-next-line @silverhand/fp/no-mutation
-    values = JSON.parse(value);
-  } catch {
-    throw new InvalidStringArrayValueError();
-  }
-
-  if (
-    !Array.isArray(values) ||
-    !values.every((value): value is string => typeof value === 'string')
-  ) {
-    throw new InvalidStringArrayValueError();
-  }
-
-  return values;
+  return rawValue
+    .split(',')
+    .map((value) => value.trim())
+    .filter(Boolean);
 };