diff --git a/.env.example b/.env.example index 1807ad1d00..021e720735 100644 --- a/.env.example +++ b/.env.example @@ -73,3 +73,9 @@ DEFAULT_GITHUB_CLIENT_SECRET= # Hosted Auth Configuration WORKOS_API_KEY= WORKOS_CLIENT_ID= + +# Encryption key for secrets / records stored in database +NANGO_ENCRYPTION_KEY= + +# Enable or disable environment keys cache +NANGO_CACHE_ENV_KEYS= diff --git a/package-lock.json b/package-lock.json index 6551e6e54a..74a16f4364 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23286,6 +23286,31 @@ "version": "2.1.3", "license": "MIT" }, + "node_modules/serialize-error": { + "version": "11.0.3", + "resolved": "https://registry.npmjs.org/serialize-error/-/serialize-error-11.0.3.tgz", + "integrity": "sha512-2G2y++21dhj2R7iHAdd0FIzjGwuKZld+7Pl/bTU6YIkrC2ZMbVUjm+luj6A6V34Rv9XfKJDKpTWu9W4Gse1D9g==", + "dependencies": { + "type-fest": "^2.12.2" + }, + "engines": { + "node": ">=14.16" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/serialize-error/node_modules/type-fest": { + "version": "2.19.0", + "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-2.19.0.tgz", + "integrity": "sha512-RAH822pAdBgcNMAfWnCBU3CFZcfZ/i1eZjwFU/dsLKumyuuP3niueg2UAukXYF0E2AAoc82ZSSf9J0WQBinzHA==", + "engines": { + "node": ">=12.20" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/serialize-javascript": { "version": "6.0.2", "resolved": "https://registry.npmjs.org/serialize-javascript/-/serialize-javascript-6.0.2.tgz", @@ -32200,6 +32225,7 @@ "redis": "^4.6.11", "rimraf": "^5.0.1", "semver": "^7.5.4", + "serialize-error": "11.0.3", "simple-oauth2": "^5.0.0", "uuid": "^9.0.0", "winston": "^3.8.2", diff --git a/packages/server/lib/controllers/access.middleware.ts b/packages/server/lib/controllers/access.middleware.ts index a187e0c510..79ac893ffa 100644 --- a/packages/server/lib/controllers/access.middleware.ts +++ b/packages/server/lib/controllers/access.middleware.ts @@ -8,7 +8,11 @@ import { setEnvironmentId, isBasicAuthEnabled, errorManager, - userService + userService, + logger, + stringifyError, + telemetry, + MetricTypes } from '@nangohq/shared'; import tracer from 'dd-trace'; @@ -32,12 +36,16 @@ export class AccessMiddleware { let accountId: number | null; let environmentId: number | null; + const start = Date.now(); try { const result = await environmentService.getAccountIdAndEnvironmentIdBySecretKey(secret); accountId = result?.accountId as number; environmentId = result?.environmentId as number; - } catch (_) { + } catch (err) { + logger.error(`failed_get_env_by_secret_key ${stringifyError(err)}`); return errorManager.errRes(res, 'malformed_auth_header'); + } finally { + telemetry.duration(MetricTypes.AUTH_GET_ENV_BY_SECRET_KEY, Date.now() - start); } if (accountId == null) { diff --git a/packages/shared/lib/db/migrations/20240326141028_hash-key.cjs b/packages/shared/lib/db/migrations/20240326141028_hash-key.cjs new file mode 100644 index 0000000000..ab5f0a16e2 --- /dev/null +++ b/packages/shared/lib/db/migrations/20240326141028_hash-key.cjs @@ -0,0 +1,36 @@ +/* eslint-disable @typescript-eslint/no-var-requires */ +const utils = require('node:util'); +const crypto = require('node:crypto'); + +const pbkdf2 = utils.promisify(crypto.pbkdf2); +const ENCRYPTION_KEY = process.env['NANGO_ENCRYPTION_KEY']; + +function decrypt(enc, iv, authTag) { + const decipher = crypto.createDecipheriv('aes-256-gcm', Buffer.from(ENCRYPTION_KEY, 'base64'), Buffer.from(iv, 'base64')); + decipher.setAuthTag(Buffer.from(authTag, 'base64')); + let str = decipher.update(enc, 'base64', 'utf8'); + str += decipher.final('utf8'); + return str; +} + +exports.up = async function (knex) { + await knex.schema.raw('ALTER TABLE "_nango_environments" ADD COLUMN IF NOT EXISTS "secret_key_hashed" varchar(64)'); + + if (!ENCRYPTION_KEY) { + return; + } + + const envs = await knex.select('*').from(`_nango_environments`).where({ secret_key_hashed: null }); + + for (const env of envs) { + const decrypted = env.secret_key_iv ? decrypt(env.secret_key, env.secret_key_iv, env.secret_key_tag) : env.secret_key; + await knex + .from(`_nango_environments`) + .where({ id: env.id }) + .update({ secret_key_hashed: (await pbkdf2(decrypted, ENCRYPTION_KEY, 310000, 32, 'sha256')).toString('base64') }); + } +}; + +exports.down = async function (knex) { + await knex.schema.raw('ALTER TABLE "_nango_environments" DROP COLUMN IF EXISTS "secret_key_hashed"'); +}; diff --git a/packages/shared/lib/db/seeders/account.seeder.ts b/packages/shared/lib/db/seeders/account.seeder.ts new file mode 100644 index 0000000000..997c2bbe24 --- /dev/null +++ b/packages/shared/lib/db/seeders/account.seeder.ts @@ -0,0 +1,12 @@ +import { v4 as uuid } from 'uuid'; +import type { Account } from '../../models'; + +import accountService from '../../services/account.service'; + +export async function createAccount(): Promise { + const acc = await accountService.createAccount(uuid()); + if (!acc) { + throw new Error('failed_to_create_account'); + } + return acc; +} diff --git a/packages/shared/lib/models/Environment.ts b/packages/shared/lib/models/Environment.ts index b832779163..bed49e708e 100644 --- a/packages/shared/lib/models/Environment.ts +++ b/packages/shared/lib/models/Environment.ts @@ -9,6 +9,7 @@ export interface Environment extends Timestamps { public_key: string; secret_key_iv?: string | null; secret_key_tag?: string | null; + secret_key_hashed?: string | null; callback_url: string | null; webhook_url: string | null; websockets_path?: string | null; diff --git a/packages/shared/lib/services/environment.service.integration.test.ts b/packages/shared/lib/services/environment.service.integration.test.ts new file mode 100644 index 0000000000..6efbb1cceb --- /dev/null +++ b/packages/shared/lib/services/environment.service.integration.test.ts @@ -0,0 +1,83 @@ +import { expect, describe, it, beforeAll } from 'vitest'; +import environmentService, { hashSecretKey } from './environment.service.js'; +import { v4 as uuid } from 'uuid'; +import { multipleMigrations } from '../db/database.js'; +import { createAccount } from '../db/seeders/account.seeder.js'; + +describe('Environment service', () => { + beforeAll(async () => { + await multipleMigrations(); + }); + + it('should create a service with secrets', async () => { + const account = await createAccount(); + const envName = uuid(); + const env = await environmentService.createEnvironment(account.id, envName); + if (!env) { + throw new Error('failed_to_create_env'); + } + + expect(env).toStrictEqual({ + account_id: account.id, + always_send_webhook: false, + callback_url: null, + created_at: expect.toBeIsoDate(), + hmac_enabled: false, + hmac_key: null, + id: expect.any(Number), + name: envName, + pending_public_key: null, + pending_secret_key: null, + pending_secret_key_iv: null, + pending_secret_key_tag: null, + public_key: expect.any(String), + secret_key: expect.any(String), + secret_key_hashed: expect.any(String), + secret_key_iv: expect.any(String), + secret_key_tag: expect.any(String), + send_auth_webhook: false, + slack_notifications: false, + updated_at: expect.toBeIsoDate(), + uuid: expect.any(String), + webhook_url: null + }); + + expect(env.secret_key).not.toEqual(env.secret_key_hashed); + }); + + it('should retrieve env by secretKey', async () => { + const account = await createAccount(); + const env = await environmentService.createEnvironment(account.id, uuid()); + + const get = await environmentService.getAccountIdAndEnvironmentIdBySecretKey(env!.secret_key); + + expect(get).toStrictEqual({ + accountId: account.id, + environmentId: env!.id + }); + }); + + it('should rotate secretKey', async () => { + const account = await createAccount(); + const env = (await environmentService.createEnvironment(account.id, uuid()))!; + expect(env.secret_key).toBeUUID(); + + // Rotate + await environmentService.rotateSecretKey(env.id); + + const env2 = (await environmentService.getById(env.id))!; + expect(env2.pending_secret_key).not.toBeNull(); + expect(env2.pending_secret_key).not.toEqual(env2.secret_key); + expect(env2.secret_key_hashed).not.toEqual(env.secret_key); + expect(env2.secret_key_hashed).toEqual(await hashSecretKey(env.secret_key)); + + // Activate + await environmentService.activateSecretKey(env.id); + + const env3 = (await environmentService.getById(env.id))!; + expect(env3.secret_key).toBeUUID(); + expect(env3.pending_secret_key).toBeNull(); + expect(env3.secret_key).toEqual(env2.pending_secret_key); + expect(env3.secret_key_hashed).toEqual(await hashSecretKey(env3.secret_key)); + }); +}); diff --git a/packages/shared/lib/services/environment.service.ts b/packages/shared/lib/services/environment.service.ts index 8f87ad25a4..ffbf4de60e 100644 --- a/packages/shared/lib/services/environment.service.ts +++ b/packages/shared/lib/services/environment.service.ts @@ -1,6 +1,6 @@ import * as uuid from 'uuid'; -import db, { schema } from '../db/database.js'; -import encryptionManager from '../utils/encryption.manager.js'; +import db from '../db/database.js'; +import encryptionManager, { ENCRYPTION_KEY, pbkdf2 } from '../utils/encryption.manager.js'; import type { Environment } from '../models/Environment.js'; import type { EnvironmentVariable } from '../models/EnvironmentVariable.js'; import type { Account } from '../models/Admin.js'; @@ -20,11 +20,16 @@ interface EnvironmentAccount { type EnvironmentAccountSecrets = Record; export const defaultEnvironments = ['prod', 'dev']; +const CACHE_ENABLED = !(process.env['NANGO_CACHE_ENV_KEYS'] === 'false'); class EnvironmentService { private environmentAccountSecrets: EnvironmentAccountSecrets = {} as EnvironmentAccountSecrets; async cacheSecrets(): Promise { + if (!CACHE_ENABLED) { + return; + } + const environmentAccounts = await db.knex.select('*').from(TABLE); const environmentAccountSecrets: EnvironmentAccountSecrets = {}; @@ -45,6 +50,10 @@ class EnvironmentService { } private addToEnvironmentSecretCache(accountEnvironment: Environment) { + if (!CACHE_ENABLED) { + return; + } + this.environmentAccountSecrets[accountEnvironment.secret_key] = { accountId: accountEnvironment.account_id, environmentId: accountEnvironment.id, @@ -96,11 +105,15 @@ class EnvironmentService { if (!this.environmentAccountSecrets[secretKey]) { // If the secret key is not in the cache, try to get it from the database - const fromDb = await db.knex.select('*').from(TABLE).where({ secret_key: secretKey }).first(); + const hashed = await hashSecretKey(secretKey); + const fromDb = await db.knex.select('*').from(TABLE).where({ secret_key_hashed: hashed }).first(); if (!fromDb) { return null; } - this.addToEnvironmentSecretCache(fromDb); + if (!CACHE_ENABLED) { + return { accountId: fromDb.account_id, environmentId: fromDb.id }; + } + this.addToEnvironmentSecretCache(encryptionManager.decryptEnvironment(fromDb)!); } const { accountId, environmentId } = this.environmentAccountSecrets[secretKey] as EnvironmentAccount; @@ -177,29 +190,6 @@ class EnvironmentService { return { accountId: result.account_id, environmentId: result.id }; } - async getByAccountIdAndEnvironment(id: number): Promise { - try { - const result = await db.knex.select('*').from(TABLE).where({ id }); - - if (result == null || result.length == 0 || result[0] == null) { - return null; - } - - return encryptionManager.decryptEnvironment(result[0]); - } catch (e) { - await errorManager.report(e, { - environmentId: id, - source: ErrorSourceEnum.PLATFORM, - operation: LogActionEnum.DATABASE, - metadata: { - id - } - }); - - return null; - } - } - async getAccountAndEnvironmentById(account_id: number, environment: string): Promise<{ account: Account | null; environment: Environment | null }> { const account = await db.knex.select('*').from(`_nango_accounts`).where({ id: account_id }); @@ -228,7 +218,7 @@ class EnvironmentService { async getById(id: number): Promise { try { - const result = (await schema().select('*').from(TABLE).where({ id })) as unknown as Environment[]; + const result = await db.knex.select('*').from(TABLE).where({ id }); if (result == null || result.length == 0 || result[0] == null) { return null; @@ -281,18 +271,24 @@ class EnvironmentService { } async createEnvironment(accountId: number, environment: string): Promise { - const result: void | Pick = await schema().from(TABLE).insert({ account_id: accountId, name: environment }, ['id']); + const result = await db.knex.from(TABLE).insert({ account_id: accountId, name: environment }).returning('id'); - if (Array.isArray(result) && result.length === 1 && result[0] != null && 'id' in result[0]) { + if (Array.isArray(result) && result.length === 1 && result[0] && 'id' in result[0]) { const environmentId = result[0]['id']; const environment = await this.getById(environmentId); - - if (environment != null) { - const encryptedEnvironment = encryptionManager.encryptEnvironment(environment); - await schema().from(TABLE).where({ id: environmentId }).update(encryptedEnvironment); - this.addToEnvironmentSecretCache(environment); - return encryptedEnvironment; + if (!environment) { + return null; } + + const encryptedEnvironment = encryptionManager.encryptEnvironment({ + ...environment, + secret_key_hashed: await hashSecretKey(environment.secret_key) + }); + await db.knex.from(TABLE).where({ id: environmentId }).update(encryptedEnvironment); + + const env = encryptionManager.decryptEnvironment(encryptedEnvironment)!; + this.addToEnvironmentSecretCache(env); + return env; } return null; @@ -500,6 +496,7 @@ class EnvironmentService { return false; } + const decrypted = encryptionManager.decryptEnvironment(environment)!; await db.knex .from(TABLE) .where({ id }) @@ -507,14 +504,15 @@ class EnvironmentService { secret_key: environment.pending_secret_key as string, secret_key_iv: environment.pending_secret_key_iv as string, secret_key_tag: environment.pending_secret_key_tag as string, + secret_key_hashed: await hashSecretKey(decrypted.pending_secret_key!), pending_secret_key: null, pending_secret_key_iv: null, pending_secret_key_tag: null }); - if (this.environmentAccountSecrets[environment.secret_key]) { + if (this.environmentAccountSecrets[decrypted.secret_key]) { // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete this.environmentAccountSecrets[environment.secret_key]; + delete this.environmentAccountSecrets[decrypted.secret_key]; } const updatedEnvironment = await this.getById(id); @@ -547,4 +545,12 @@ class EnvironmentService { } } +export async function hashSecretKey(key: string) { + if (!ENCRYPTION_KEY) { + return key; + } + + return (await pbkdf2(key, ENCRYPTION_KEY, 310000, 32, 'sha256')).toString('base64'); +} + export default new EnvironmentService(); diff --git a/packages/shared/lib/services/environment.service.unit.test.ts b/packages/shared/lib/services/environment.service.unit.test.ts deleted file mode 100644 index 815099f0ab..0000000000 --- a/packages/shared/lib/services/environment.service.unit.test.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { expect, describe, it, vi } from 'vitest'; -import environmentService from './environment.service.js'; -import type { Environment } from '../models/Environment.js'; -import encryptionManager from '../utils/encryption.manager.js'; - -const environmentAccounts = [ - { id: 1, account_id: '123', name: 'env1', secret_key: 'key1' }, - { id: 2, account_id: '456', name: 'env2', secret_key: 'key2' }, - { id: 3, account_id: '789', name: 'env3', secret_key: 'key3' } -]; -const extraAccount = { id: 4, account_id: '1011', name: 'env4', secret_key: 'key4' }; - -describe('Environment service', () => { - it('can retrieve secrets', async () => { - vi.mock('../db/database.js', async () => { - const actual = (await vi.importActual('../db/database.js')) as any; - return { - default: { - schema: actual.schema, - knex: { - select: () => { - return { - from: () => - new Proxy( - { - where: ({ secret_key }: { secret_key: string }) => { - return { - first: () => { - if (secret_key === extraAccount.secret_key) { - return Promise.resolve(extraAccount); - } else { - return Promise.resolve(null); - } - } - }; - } - }, - { - get(target, prop, receiver) { - if (prop === Symbol.iterator) { - return function* () { - yield* environmentAccounts; - }; - } - return Reflect.get(target, prop, receiver); - } - } - ) - }; - } - } - } - }; - }); - vi.spyOn(encryptionManager, 'decryptEnvironment').mockImplementation((environmentAccount: Environment | null) => { - return environmentAccount; - }); - vi.spyOn(environmentService as any, 'addToEnvironmentSecretCache'); - - await environmentService.cacheSecrets(); - expect(await environmentService.getAccountIdAndEnvironmentIdBySecretKey('key1')).toEqual({ accountId: '123', environmentId: 1 }); - expect(await environmentService.getAccountIdAndEnvironmentIdBySecretKey('key2')).toEqual({ accountId: '456', environmentId: 2 }); - expect(await environmentService.getAccountIdAndEnvironmentIdBySecretKey('key3')).toEqual({ accountId: '789', environmentId: 3 }); - expect(await environmentService.getAccountIdAndEnvironmentIdBySecretKey('key4')).toEqual({ accountId: '1011', environmentId: 4 }); - expect(await environmentService.getAccountIdAndEnvironmentIdBySecretKey('key5')).toEqual(null); - - // retrieving the same key again should not call addToEnvironmentSecretCache again - expect(await environmentService.getAccountIdAndEnvironmentIdBySecretKey('key4')).toEqual({ accountId: '1011', environmentId: 4 }); - expect((environmentService as any).addToEnvironmentSecretCache).toHaveBeenCalledTimes(1); - }); -}); diff --git a/packages/shared/lib/utils/encryption.manager.ts b/packages/shared/lib/utils/encryption.manager.ts index d4c0baac32..b07777aad1 100644 --- a/packages/shared/lib/utils/encryption.manager.ts +++ b/packages/shared/lib/utils/encryption.manager.ts @@ -1,4 +1,5 @@ import type { CipherGCMTypes } from 'crypto'; +import utils from 'node:util'; import crypto from 'crypto'; import logger from '../logger/console.js'; import type { Config as ProviderConfig } from '../models/Provider'; @@ -8,13 +9,15 @@ import type { EnvironmentVariable } from '../models/EnvironmentVariable.js'; import type { Connection, ApiConnection, StoredConnection } from '../models/Connection.js'; import type { RawDataRecordResult, DataRecord, DataRecordWithMetadata, RecordWrapCustomerFacingDataRecord, UnencryptedRawRecord } from '../models/Sync.js'; import db from '../db/database.js'; -import util from 'util'; interface DataRecordJson { encryptedValue: string; [key: string]: any; } +export const pbkdf2 = utils.promisify(crypto.pbkdf2); +export const ENCRYPTION_KEY = process.env['NANGO_ENCRYPTION_KEY']; + class EncryptionManager { private key: string | undefined; private algo: CipherGCMTypes = 'aes-256-gcm'; @@ -25,7 +28,7 @@ class EncryptionManager { constructor(key: string | undefined) { this.key = key; - if (key && Buffer.from(key, this.encoding).byteLength != this.encryptionKeyByteLength) { + if (key && Buffer.from(key, this.encoding).byteLength !== this.encryptionKeyByteLength) { throw new Error('Encryption key must be base64-encoded and 256-bit long.'); } } @@ -331,7 +334,7 @@ class EncryptionManager { } private async hashEncryptionKey(key: string, salt: string): Promise { - const keyBuffer = await util.promisify(crypto.pbkdf2)(key, salt, 310000, 32, 'sha256'); + const keyBuffer = await pbkdf2(key, salt, 310000, 32, 'sha256'); return keyBuffer.toString(this.encoding); } @@ -422,4 +425,4 @@ class EncryptionManager { } } -export default new EncryptionManager(process.env['NANGO_ENCRYPTION_KEY']); +export default new EncryptionManager(ENCRYPTION_KEY); diff --git a/packages/shared/lib/utils/error.ts b/packages/shared/lib/utils/error.ts index 894fa3729a..f80d8708ab 100644 --- a/packages/shared/lib/utils/error.ts +++ b/packages/shared/lib/utils/error.ts @@ -1,3 +1,5 @@ +import { serializeError } from 'serialize-error'; + export class NangoError extends Error { public readonly status: number = 500; public readonly type: string; @@ -588,3 +590,7 @@ export const formatScriptError = (err: any, errorType: string, scriptName: strin return { success: false, error, response: null }; }; + +export function stringifyError(err: unknown) { + return JSON.stringify(serializeError(err)); +} diff --git a/packages/shared/lib/utils/telemetry.ts b/packages/shared/lib/utils/telemetry.ts index c8728c3e5a..35cab764de 100644 --- a/packages/shared/lib/utils/telemetry.ts +++ b/packages/shared/lib/utils/telemetry.ts @@ -48,7 +48,8 @@ export enum MetricTypes { JOBS_DELETE_SYNCS_DATA_RECORDS = 'nango.jobs.cron.deleteSyncsData.records', JOBS_DELETE_SYNCS_DATA_DELETES = 'nango.jobs.cron.deleteSyncsData.deletes', PERSIST_RECORDS_COUNT = 'nango.persist.records.count', - PERSIST_RECORDS_SIZE_IN_BYTES = 'nango.persist.records.sizeInBytes' + PERSIST_RECORDS_SIZE_IN_BYTES = 'nango.persist.records.sizeInBytes', + AUTH_GET_ENV_BY_SECRET_KEY = 'nango.auth.getEnvBySecretKey' } export enum SpanTypes { diff --git a/packages/shared/lib/utils/utils.ts b/packages/shared/lib/utils/utils.ts index 931e608683..aca503bab9 100644 --- a/packages/shared/lib/utils/utils.ts +++ b/packages/shared/lib/utils/utils.ts @@ -240,7 +240,7 @@ export async function getOauthCallbackUrl(environmentId?: number) { const globalCallbackUrl = getGlobalOAuthCallbackUrl(); if (environmentId != null) { - const environment: Environment | null = await environmentService.getByAccountIdAndEnvironment(environmentId); + const environment: Environment | null = await environmentService.getById(environmentId); return environment?.callback_url || globalCallbackUrl; } diff --git a/packages/shared/package.json b/packages/shared/package.json index 719b833253..21b2aaef88 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -52,6 +52,7 @@ "redis": "^4.6.11", "rimraf": "^5.0.1", "semver": "^7.5.4", + "serialize-error": "11.0.3", "simple-oauth2": "^5.0.0", "uuid": "^9.0.0", "winston": "^3.8.2", diff --git a/vite.config.ts b/vite.config.ts index a0c2efc132..98f2627d6a 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -2,7 +2,7 @@ // Configure Vitest (https://vitest.dev/config/) -import { defineConfig } from 'vite'; +import { defineConfig } from 'vitest/config'; export default defineConfig({ test: { diff --git a/vite.integration.config.ts b/vite.integration.config.ts index bed8c12869..a95f752a1c 100644 --- a/vite.integration.config.ts +++ b/vite.integration.config.ts @@ -1,7 +1,7 @@ /// // Configure Vitest (https://vitest.dev/config/) -import { defineConfig } from 'vite'; +import { defineConfig } from 'vitest/config'; process.env.TZ = 'UTC'; @@ -11,6 +11,9 @@ export default defineConfig({ globalSetup: './tests/setup.ts', setupFiles: './tests/setupFiles.ts', threads: false, - testTimeout: 20000 + testTimeout: 20000, + env: { + NANGO_ENCRYPTION_KEY: 'RzV4ZGo5RlFKMm0wYWlXdDhxTFhwb3ZrUG5KNGg3TmU=' + } } });