From b178b7eb25b16240524520307e2e94aa64e7f3c7 Mon Sep 17 00:00:00 2001 From: Pranav C Date: Sun, 6 Oct 2024 07:21:34 +0000 Subject: [PATCH] refactor: coderabbit review comments --- packages/nc-secret-cli/src/core/NcConfig.ts | 12 ++------- packages/nc-secret-cli/src/core/NcLogger.ts | 27 ------------------- .../nc-secret-cli/src/core/SecretManager.ts | 8 +++--- packages/nc-secret-cli/src/core/index.ts | 4 +-- packages/nc-secret-cli/src/core/logger.ts | 25 +++++++++++++++++ packages/nc-secret-cli/src/index.ts | 9 +++---- .../100.data-sources/050.updating-secret.md | 4 +-- packages/nocodb/src/cli.ts | 6 ++++- .../src/helpers/initDataSourceEncryption.ts | 5 ++-- .../nocodb/src/models/Integration.spec.ts | 6 ++--- packages/nocodb/src/models/Integration.ts | 5 ++-- packages/nocodb/src/models/Source.ts | 2 +- .../providers/init-meta-service.provider.ts | 2 +- .../base-users/base-users.service.spec.ts | 2 +- .../nocodb/src/utils/nc-config/NcConfig.ts | 5 ---- .../upgraders/0225002_ncDatasourceDecrypt.ts | 1 - 16 files changed, 55 insertions(+), 68 deletions(-) delete mode 100644 packages/nc-secret-cli/src/core/NcLogger.ts create mode 100644 packages/nc-secret-cli/src/core/logger.ts diff --git a/packages/nc-secret-cli/src/core/NcConfig.ts b/packages/nc-secret-cli/src/core/NcConfig.ts index db190c2b0e..a3c04a0ee9 100644 --- a/packages/nc-secret-cli/src/core/NcConfig.ts +++ b/packages/nc-secret-cli/src/core/NcConfig.ts @@ -17,8 +17,6 @@ export class NcConfig { toolDir: string; - credentialSecret?: string; - private constructor() { this.toolDir = getToolDir(); } @@ -32,17 +30,12 @@ export class NcConfig { databaseUrl?: string; }; secret?: string; - credentialSecret?: string; }): Promise { const { meta, secret } = param; const ncConfig = new NcConfig(); - - ncConfig.credentialSecret = param.credentialSecret; - - if (ncConfig.meta?.db?.connection?.filename) { ncConfig.meta.db.connection.filename = path.join( ncConfig.toolDir, @@ -76,7 +69,6 @@ export class NcConfig { metaJsonFile: process.env.NC_DB_JSON_FILE, }, secret: process.env.NC_AUTH_JWT_SECRET, - credentialSecret: process.env.NC_KEY_CREDENTIAL_ENCRYPT, }); } } @@ -90,8 +82,8 @@ export const getNocoConfig = async (options: { } ={}) =>{ // check for JDBC url specified in env or options await prepareEnv({ - databaseUrl: options.ncDatabaseUrl || process.env.NC_DATABASE_URL || process.env.DATABASE_URL, - databaseUrlFile: options.ncDatabaseUrlFile || process.env.NC_DATABASE_URL_FILE || process.env.DATABASE_URL_FILE, + databaseUrl: options.databaseUrl || process.env.NC_DATABASE_URL || process.env.DATABASE_URL, + databaseUrlFile: options.databaseUrlFile || process.env.NC_DATABASE_URL_FILE || process.env.DATABASE_URL_FILE, }) // create NocoConfig using utility method which works similar to Nocodb NcConfig with only meta db config diff --git a/packages/nc-secret-cli/src/core/NcLogger.ts b/packages/nc-secret-cli/src/core/NcLogger.ts deleted file mode 100644 index 9da4268c75..0000000000 --- a/packages/nc-secret-cli/src/core/NcLogger.ts +++ /dev/null @@ -1,27 +0,0 @@ -// a class to log messages to the console with colors and styles -export class NcLogger { - static log(message: string) { - console.log(message); - } - - static error(message: string) { - console.error('\x1b[31m%s\x1b[0m', 'Error: ' + message); - } - - static warn(message: string) { - console.warn('\x1b[33m%s\x1b[0m', 'Warning: ' + message); - } - - static info(message: string) { - console.info('\x1b[32m%s\x1b[0m', 'Info: ' + message); - } - - static success(message: string) { - console.log('\x1b[32m%s\x1b[0m', 'Success: ' + message); - } - - static debug(message: string) { - console.debug('\x1b[34m%s\x1b[0m', 'Debug: ' + message); - } - -} \ No newline at end of file diff --git a/packages/nc-secret-cli/src/core/SecretManager.ts b/packages/nc-secret-cli/src/core/SecretManager.ts index 723b0b4a0d..f4bd064d0f 100644 --- a/packages/nc-secret-cli/src/core/SecretManager.ts +++ b/packages/nc-secret-cli/src/core/SecretManager.ts @@ -1,5 +1,5 @@ import {NcError} from "./NcError"; -import {NcLogger} from "./NcLogger"; +import * as logger from "./logger"; const { SqlClientFactory, MetaTable, decryptPropIfRequired, encryptPropIfRequired } = require('../nocodb/cli') @@ -17,7 +17,7 @@ export class SecretManager { // use the sqlClientFactory to create a new sql client and then use testConnection to test the connection const isValid = await this.sqlClient.testConnection(); if (!isValid) { - throw new NcError('Invalid database configuration'); + throw new NcError('Invalid database configuration. Please verify your database settings and ensure the database is reachable.'); } } @@ -61,7 +61,7 @@ export class SecretManager { isValid = true; sourcesToUpdate.push({ ...source, config: decrypted }); } catch (e) { - NcLogger.error('Failed to decrypt source configuration : ' + e.message); + logger.error('Failed to decrypt source configuration : ' + e.message); } } @@ -79,7 +79,7 @@ export class SecretManager { } } - // if all of the decyptions are failed then throw an error + // If all decryptions have failed, then throw an error if (!isValid) { throw new NcError('Invalid old secret or no sources/integrations found'); } diff --git a/packages/nc-secret-cli/src/core/index.ts b/packages/nc-secret-cli/src/core/index.ts index 9dddd203fe..d190da4c29 100644 --- a/packages/nc-secret-cli/src/core/index.ts +++ b/packages/nc-secret-cli/src/core/index.ts @@ -1,4 +1,4 @@ export * from './NcConfig'; export * from './NcError'; -export * from './NcLogger'; -export * from './SecretManager'; \ No newline at end of file +export * as logger from './logger'; +export * from './SecretManager'; diff --git a/packages/nc-secret-cli/src/core/logger.ts b/packages/nc-secret-cli/src/core/logger.ts new file mode 100644 index 0000000000..d8ce0e797f --- /dev/null +++ b/packages/nc-secret-cli/src/core/logger.ts @@ -0,0 +1,25 @@ +import chalk from 'chalk'; + + export function log(message: string) { + console.log(chalk.white(message)); + } + + export function error(message: string) { + console.error(chalk.red('Error: ' + message)); + } + + export function warn(message: string) { + console.warn(chalk.yellow('Warning: ' + message)); + } + + export function info(message: string) { + console.info(chalk.green('Info: ' + message)); + } + + export function success(message: string) { + console.log(chalk.green('Success: ' + message)); + } + + export function debug(message: string) { + console.debug(chalk.blue('Debug: ' + message)); + } diff --git a/packages/nc-secret-cli/src/index.ts b/packages/nc-secret-cli/src/index.ts index 5c13d60b2b..2d908d7f7e 100644 --- a/packages/nc-secret-cli/src/index.ts +++ b/packages/nc-secret-cli/src/index.ts @@ -3,7 +3,7 @@ import { Command } from 'commander'; import { getNocoConfig } from "./core"; import { SecretManager } from "./core"; import { NcError } from "./core"; -import { NcLogger } from "./core"; +import { logger } from "./core"; console.log(figlet.textSync("NocoDB Secret CLI")); @@ -18,8 +18,8 @@ program .option('--nc-db-json-file ', 'NocoDB connection database json file path, equivalent to NC_DB_JSON_FILE env variable') .option('--database-url ', 'JDBC database url, equivalent to DATABASE_URL env variable') .option('--database-url-file ', 'JDBC database url file path, equivalent to DATABASE_URL_FILE env variable') - .option('-o, --old-secret ', 'old secret string to decrypt sources and integrations') - .option('-n, --new-secret ', 'new secret string to encrypt sources and integrations') + .option('-p, --prev ', 'old secret string to decrypt sources and integrations') + .option('-n, --new ', 'new secret string to encrypt sources and integrations') .action(async (prevVal, newVal) => { try { @@ -48,8 +48,7 @@ program } catch (e) { if (e instanceof NcError) { // print error message in a better way - NcLogger.error(e.message); - + logger.error(e.message); process.exit(1); } console.error(e); diff --git a/packages/noco-docs/docs/100.data-sources/050.updating-secret.md b/packages/noco-docs/docs/100.data-sources/050.updating-secret.md index 0fa31f7d4f..ad5a8771db 100644 --- a/packages/noco-docs/docs/100.data-sources/050.updating-secret.md +++ b/packages/noco-docs/docs/100.data-sources/050.updating-secret.md @@ -29,7 +29,7 @@ To update a secret in NocoDB, you can use the `nc-secret-cli` package. Follow th NC_DB="pg://host:port?u=user&p=password&d=database" nc-secret-cli ``` - Replace `` with the name of the secret you used previously, and `` with the new value of the secret. + Replace `` with the name of the secret you used previously, and `` with the new value of the secret. 3. After running the command, the secret will be updated in NocoDB. @@ -44,7 +44,7 @@ Alternatively, you can use the `nc-secret-cli` executable to update secrets. NC_DB="pg://host:port?u=user&p=password&d=database" ./nc-secret-macos-arm64 update --prev --new ``` - Replace `` with the name of the secret you used previously, and `` with the new value of the secret. + Replace `` with the name of the secret you used previously, and `` with the new value of the secret. 3. After running the command, the secret will be updated in NocoDB. diff --git a/packages/nocodb/src/cli.ts b/packages/nocodb/src/cli.ts index 9a3098a9f7..661dc8d427 100644 --- a/packages/nocodb/src/cli.ts +++ b/packages/nocodb/src/cli.ts @@ -1,5 +1,9 @@ export { SqlClientFactory } from '~/db/sql-client/lib/SqlClientFactory'; export { MetaTable } from '~/utils/globals'; export * from '~/utils/encryptDecrypt'; -export { getToolDir, metaUrlToDbConfig, prepareEnv } from '~/utils/nc-config/helpers'; +export { + getToolDir, + metaUrlToDbConfig, + prepareEnv, +} from '~/utils/nc-config/helpers'; export { DriverClient } from '~/utils/nc-config/constants'; diff --git a/packages/nocodb/src/helpers/initDataSourceEncryption.ts b/packages/nocodb/src/helpers/initDataSourceEncryption.ts index 53ffbd18ad..c6e984786c 100644 --- a/packages/nocodb/src/helpers/initDataSourceEncryption.ts +++ b/packages/nocodb/src/helpers/initDataSourceEncryption.ts @@ -1,4 +1,3 @@ -import process from 'process'; import Noco from '~/Noco'; import { MetaTable, RootScopes } from '~/utils/globals'; import { encryptPropIfRequired } from '~/utils'; @@ -70,7 +69,7 @@ export default async function initDataSourceEncryption(_ncMeta = Noco.ncMeta) { await ncMeta.commit(); } catch (e) { await ncMeta.rollback(); - console.error('Failed to encrypt data sources', e); - process.exit(1); + console.error('Failed to encrypt data sources'); + throw e; } } diff --git a/packages/nocodb/src/models/Integration.spec.ts b/packages/nocodb/src/models/Integration.spec.ts index 45f58545ec..1e75baa750 100644 --- a/packages/nocodb/src/models/Integration.spec.ts +++ b/packages/nocodb/src/models/Integration.spec.ts @@ -3,8 +3,6 @@ import { Integration } from '~/models'; import { MetaTable } from '~/utils/globals'; import { decryptPropIfRequired } from '~/utils'; -// Mock dependencies -jest.mock('~/helpers/catchError'); jest.mock('~/Noco'); describe('Integration Model', () => { @@ -196,7 +194,9 @@ describe('Integration Model', () => { const calledWithArgs = mockNcMeta.metaInsert2.mock.calls[0][3]; // veify the 'config' field is encrypted - expect(calledWithArgs.config).not.toEqual(JSON.stringify(newIntegration.config)); + expect(calledWithArgs.config).not.toEqual( + JSON.stringify(newIntegration.config), + ); // Decrypt the 'config' field const decryptedConfig = decryptPropIfRequired({ data: calledWithArgs }); diff --git a/packages/nocodb/src/models/Integration.ts b/packages/nocodb/src/models/Integration.ts index dfbd390640..fef0660f42 100644 --- a/packages/nocodb/src/models/Integration.ts +++ b/packages/nocodb/src/models/Integration.ts @@ -12,7 +12,8 @@ import { } from '~/utils/modelUtils'; import { decryptPropIfRequired, - encryptPropIfRequired, isEncryptionRequired, + encryptPropIfRequired, + isEncryptionRequired, partialExtract, } from '~/utils'; import { PagedResponseImpl } from '~/helpers/PagedResponse'; @@ -129,7 +130,7 @@ export default class Integration implements IntegrationType { 'deleted', 'config', 'is_private', - 'is_encrypted' + 'is_encrypted', ]); if (updateObj.config) { diff --git a/packages/nocodb/src/models/Source.ts b/packages/nocodb/src/models/Source.ts index f4bd970cdd..b5688e5161 100644 --- a/packages/nocodb/src/models/Source.ts +++ b/packages/nocodb/src/models/Source.ts @@ -70,7 +70,7 @@ export default class Source implements SourceType { created_at?; updated_at?; meta?: any; - is_encrypted?: any; + is_encrypted?: boolean; }, ncMeta = Noco.ncMeta, ) { diff --git a/packages/nocodb/src/providers/init-meta-service.provider.ts b/packages/nocodb/src/providers/init-meta-service.provider.ts index ca5b00b11c..27e2512e1d 100644 --- a/packages/nocodb/src/providers/init-meta-service.provider.ts +++ b/packages/nocodb/src/providers/init-meta-service.provider.ts @@ -14,7 +14,7 @@ import { NcConfig, prepareEnv } from '~/utils/nc-config'; import { MetaTable, RootScopes } from '~/utils/globals'; import { updateMigrationJobsState } from '~/helpers/migrationJobs'; import { initBaseBehavior } from '~/helpers/initBaseBehaviour'; -import initDataSourceEncryption from "~/helpers/initDataSourceEncryption"; +import initDataSourceEncryption from '~/helpers/initDataSourceEncryption'; export const InitMetaServiceProvider: FactoryProvider = { // initialize app, diff --git a/packages/nocodb/src/services/base-users/base-users.service.spec.ts b/packages/nocodb/src/services/base-users/base-users.service.spec.ts index c1c9d70742..e63285b5df 100644 --- a/packages/nocodb/src/services/base-users/base-users.service.spec.ts +++ b/packages/nocodb/src/services/base-users/base-users.service.spec.ts @@ -1,9 +1,9 @@ import { Test } from '@nestjs/testing'; import { mock } from 'jest-mock-extended'; import type { TestingModule } from '@nestjs/testing'; +import type { IEventEmitter } from '~/modules/event-emitter/event-emitter.interface'; import { BaseUsersService } from '~/services/base-users/base-users.service'; import { AppHooksService } from '~/services/app-hooks/app-hooks.service'; -import { IEventEmitter } from '~/modules/event-emitter/event-emitter.interface'; describe('BaseUsersService', () => { let service: BaseUsersService; diff --git a/packages/nocodb/src/utils/nc-config/NcConfig.ts b/packages/nocodb/src/utils/nc-config/NcConfig.ts index 7e52a1a510..40de298af8 100644 --- a/packages/nocodb/src/utils/nc-config/NcConfig.ts +++ b/packages/nocodb/src/utils/nc-config/NcConfig.ts @@ -44,7 +44,6 @@ export class NcConfig { env: string; workingEnv: string; baseType: string; - credentialSecret?: string; private constructor() {} @@ -60,7 +59,6 @@ export class NcConfig { worker?: boolean; dashboardPath?: string; publicUrl?: string; - credentialSecret?: string; }): Promise { const { meta, secret, port, worker, tryMode, publicUrl, dashboardPath } = param; @@ -73,8 +71,6 @@ export class NcConfig { }, }; - ncConfig.credentialSecret = param.credentialSecret; - ncConfig.port = +(port ?? 8080); ncConfig.toolDir = getToolDir(); ncConfig.worker = worker ?? false; @@ -152,7 +148,6 @@ export class NcConfig { worker: !!process.env.NC_WORKER, dashboardPath: process.env.NC_DASHBOARD_URL ?? '/dashboard', publicUrl: process.env.NC_PUBLIC_URL, - credentialSecret: process.env.NC_KEY_CREDENTIAL_ENCRYPT, }); } diff --git a/packages/nocodb/src/version-upgrader/upgraders/0225002_ncDatasourceDecrypt.ts b/packages/nocodb/src/version-upgrader/upgraders/0225002_ncDatasourceDecrypt.ts index 108a25659f..b5789a233f 100644 --- a/packages/nocodb/src/version-upgrader/upgraders/0225002_ncDatasourceDecrypt.ts +++ b/packages/nocodb/src/version-upgrader/upgraders/0225002_ncDatasourceDecrypt.ts @@ -1,4 +1,3 @@ -import process from 'process'; import CryptoJS from 'crypto-js'; import type { NcUpgraderCtx } from '~/version-upgrader/NcUpgrader'; import { MetaTable, RootScopes } from '~/utils/globals';