From f6db981600fd16a860262336ad88d886ca502628 Mon Sep 17 00:00:00 2001 From: Xiao Yijun Date: Thu, 11 Aug 2022 19:43:55 +0800 Subject: [PATCH] feat(core)!: use comma separated values as a string array in the env file (#1762) --- packages/core/jest.setup.ts | 3 +- packages/core/src/connectors/consts.ts | 10 +-- packages/core/src/connectors/index.ts | 9 ++- packages/core/src/env-set/index.ts | 3 + packages/core/src/env-set/oidc.test.ts | 91 ++++++++++++++------------ packages/core/src/env-set/oidc.ts | 36 ++++------ packages/core/src/env-set/utils.ts | 44 ++----------- 7 files changed, 80 insertions(+), 116 deletions(-) diff --git a/packages/core/jest.setup.ts b/packages/core/jest.setup.ts index 842661c2e..ead6df860 100644 --- a/packages/core/jest.setup.ts +++ b/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(); diff --git a/packages/core/src/connectors/consts.ts b/packages/core/src/connectors/consts.ts index e572b9199..892887337 100644 --- a/packages/core/src/connectors/consts.ts +++ b/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]; diff --git a/packages/core/src/connectors/index.ts b/packages/core/src/connectors/index.ts index 9b3cae99a..0b33cd3d7 100644 --- a/packages/core/src/connectors/index.ts +++ b/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) => { diff --git a/packages/core/src/env-set/index.ts b/packages/core/src/env-set/index.ts index 42afb4395..fe6457ae6 100644 --- a/packages/core/src/env-set/index.ts +++ b/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()), diff --git a/packages/core/src/env-set/oidc.test.ts b/packages/core/src/env-set/oidc.test.ts index 7f3f1d025..df1e18adb 100644 --- a/packages/core/src/env-set/oidc.test.ts +++ b/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(); + }); }); diff --git a/packages/core/src/env-set/oidc.ts b/packages/core/src/env-set/oidc.ts index bf807577b..f71c3abfc 100644 --- a/packages/core/src/env-set/oidc.ts +++ b/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 => { 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 => { 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 => { * * @returns The cookie keys in array. */ -const readCookieKeys = async (): Promise => { +export const readCookieKeys = async (): Promise => { const envKey = 'OIDC_COOKIE_KEYS'; const keys = getEnvAsStringArray(envKey); @@ -119,7 +111,7 @@ const readCookieKeys = async (): Promise => { } 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 => { } } - const generated = [nanoid()]; - appendDotEnv(envKey, JSON.stringify(generated)); + const generated = nanoid(); + appendDotEnv(envKey, generated); - return generated; + return [generated]; }; const loadOidcValues = async (issuer: string) => { diff --git a/packages/core/src/env-set/utils.ts b/packages/core/src/env-set/utils.ts index c2687ac71..005866ccb 100644 --- a/packages/core/src/env-set/utils.ts +++ b/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); };