-
Notifications
You must be signed in to change notification settings - Fork 368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(api): get env by secretKey #1908
Changes from 3 commits
c8d4057
b677a1e
91b83c1
807bf66
d454df3
dfd2943
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 = decrypt(env.secret_key, env.secret_key_iv, env.secret_key_tag); | ||
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"'); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<Account> { | ||
const acc = await accountService.createAccount(uuid()); | ||
if (!acc) { | ||
throw new Error('failed_to_create_account'); | ||
} | ||
return acc; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
import { expect, describe, it, beforeAll } from 'vitest'; | ||
import environmentService 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: null, | ||
secret_key_tag: null, | ||
send_auth_webhook: false, | ||
slack_notifications: false, | ||
updated_at: expect.toBeIsoDate(), | ||
uuid: expect.any(String), | ||
webhook_url: null | ||
}); | ||
// In non-encrypted env both should equal | ||
expect(env.secret_key).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()))!; | ||
|
||
// 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).toEqual(env.secret_key); | ||
|
||
// Activate | ||
await environmentService.activateSecretKey(env.id); | ||
|
||
const env3 = (await environmentService.getById(env.id))!; | ||
expect(env3.pending_secret_key).toBeNull(); | ||
expect(env3.secret_key).toEqual(env2.pending_secret_key); | ||
expect(env3.secret_key_hashed).toEqual(env2.pending_secret_key); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<string, EnvironmentAccount>; | ||
|
||
export const defaultEnvironments = ['prod', 'dev']; | ||
const CACHE_ENABLED = !(process.env['NANGO_CACHE_ENV_KEYS'] === 'false'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we have a feature flag util (reading keys from redis)
If you want to be able to enable/disable it live you would need to keep adding to the cache and check the flag only when retrieving the data https://github.com/NangoHQ/nango/pull/1908/files#diff-f3d9e285186619d6dfde44bdbbb720119863b18a3a80a41025f4a9c83e13705aL97 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, I wasn't aware of this feature. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you are worried about skewing the telemetry. You can check the flag outside of the tracking and pass the boolean to |
||
|
||
class EnvironmentService { | ||
private environmentAccountSecrets: EnvironmentAccountSecrets = {} as EnvironmentAccountSecrets; | ||
|
||
async cacheSecrets(): Promise<void> { | ||
if (!CACHE_ENABLED) { | ||
return; | ||
} | ||
|
||
const environmentAccounts = await db.knex.select('*').from<Environment>(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,10 +105,14 @@ 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<Environment>(TABLE).where({ secret_key: secretKey }).first(); | ||
const hashed = await hashSecretKey(secretKey); | ||
const fromDb = await db.knex.select('*').from<Environment>(TABLE).where({ secret_key_hashed: hashed }).first(); | ||
if (!fromDb) { | ||
return null; | ||
} | ||
if (!CACHE_ENABLED) { | ||
return { accountId: fromDb.account_id, environmentId: fromDb.id }; | ||
} | ||
this.addToEnvironmentSecretCache(fromDb); | ||
} | ||
|
||
|
@@ -177,29 +190,6 @@ class EnvironmentService { | |
return { accountId: result.account_id, environmentId: result.id }; | ||
} | ||
|
||
async getByAccountIdAndEnvironment(id: number): Promise<Environment | null> { | ||
try { | ||
const result = await db.knex.select('*').from<Environment>(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<Account>(`_nango_accounts`).where({ id: account_id }); | ||
|
||
|
@@ -228,7 +218,7 @@ class EnvironmentService { | |
|
||
async getById(id: number): Promise<Environment | null> { | ||
try { | ||
const result = (await schema().select('*').from<Environment>(TABLE).where({ id })) as unknown as Environment[]; | ||
const result = await db.knex.select('*').from<Environment>(TABLE).where({ id }); | ||
|
||
if (result == null || result.length == 0 || result[0] == null) { | ||
return null; | ||
|
@@ -281,18 +271,20 @@ class EnvironmentService { | |
} | ||
|
||
async createEnvironment(accountId: number, environment: string): Promise<Environment | null> { | ||
const result: void | Pick<Environment, 'id'> = await schema().from<Environment>(TABLE).insert({ account_id: accountId, name: environment }, ['id']); | ||
const result = await db.knex.from<Environment>(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<Environment>(TABLE).where({ id: environmentId }).update(encryptedEnvironment); | ||
this.addToEnvironmentSecretCache(environment); | ||
return encryptedEnvironment; | ||
if (!environment) { | ||
return null; | ||
} | ||
|
||
const encryptedEnvironment = encryptionManager.encryptEnvironment(environment); | ||
const newEnv = { ...encryptedEnvironment, secret_key_hashed: await hashSecretKey(environment.secret_key) }; | ||
await db.knex.from<Environment>(TABLE).where({ id: environmentId }).update(newEnv); | ||
this.addToEnvironmentSecretCache(environment); | ||
return newEnv; | ||
} | ||
|
||
return null; | ||
|
@@ -507,6 +499,7 @@ 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(environment.pending_secret_key!), | ||
pending_secret_key: null, | ||
pending_secret_key_iv: null, | ||
pending_secret_key_tag: null | ||
|
@@ -547,4 +540,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'); | ||
bodinsamuel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
export default new EnvironmentService(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because those file are .cjs we can't really import our own code. It's either work locally but not in test, or the opposite :/