diff --git a/packages/server/lib/controllers/connection.controller.ts b/packages/server/lib/controllers/connection.controller.ts index a693a7f81c0..ffd8b3aa8f1 100644 --- a/packages/server/lib/controllers/connection.controller.ts +++ b/packages/server/lib/controllers/connection.controller.ts @@ -47,11 +47,29 @@ class ConnectionController { metrics.increment(metrics.Types.GET_CONNECTION, 1, { accountId: account.id }); } - const credentialResponse = await connectionService.getConnectionCredentials({ + const integration = await configService.getProviderConfig(providerConfigKey, environment.id); + if (!integration) { + res.status(404).send({ + error: { + code: 'unknown_provider_config', + message: + 'Provider config not found for the given provider config key. Please make sure the provider config exists in the Nango dashboard.' + } + }); + return; + } + + const connectionRes = await connectionService.getConnection(connectionId, providerConfigKey, environment.id); + if (connectionRes.error || !connectionRes.response) { + errorManager.errResFromNangoErr(res, connectionRes.error); + return; + } + + const credentialResponse = await connectionService.refreshOrTestCredentials({ account, environment, - connectionId, - providerConfigKey, + connection: connectionRes.response, + integration, logContextGetter, instantRefresh, onRefreshSuccess: connectionRefreshSuccessHook, @@ -60,7 +78,6 @@ class ConnectionController { if (credentialResponse.isErr()) { errorManager.errResFromNangoErr(res, credentialResponse.error); - return; } diff --git a/packages/server/lib/controllers/proxy.controller.ts b/packages/server/lib/controllers/proxy.controller.ts index 40dce14a842..82aac8f9225 100644 --- a/packages/server/lib/controllers/proxy.controller.ts +++ b/packages/server/lib/controllers/proxy.controller.ts @@ -9,7 +9,7 @@ import querystring from 'querystring'; import type { AxiosError, AxiosRequestConfig, AxiosResponse } from 'axios'; import { backOff } from 'exponential-backoff'; import type { HTTP_VERB, UserProvidedProxyConfiguration, InternalProxyConfiguration, ApplicationConstructedProxyConfiguration, File } from '@nangohq/shared'; -import { NangoError, LogActionEnum, errorManager, ErrorSourceEnum, proxyService, connectionService, configService, featureFlags } from '@nangohq/shared'; +import { LogActionEnum, errorManager, ErrorSourceEnum, proxyService, connectionService, configService, featureFlags } from '@nangohq/shared'; import { metrics, getLogger, axiosInstance as axios, getHeaders } from '@nangohq/utils'; import { logContextGetter } from '@nangohq/logs'; import { connectionRefreshFailed as connectionRefreshFailedHook, connectionRefreshSuccess as connectionRefreshSuccessHook } from '../hooks/hooks.js'; @@ -84,11 +84,34 @@ class ProxyController { retryOn }; - const credentialResponse = await connectionService.getConnectionCredentials({ + const integration = await configService.getProviderConfig(providerConfigKey, environment.id); + if (!integration) { + await logCtx.error('Provider configuration not found'); + await logCtx.failed(); + metrics.increment(metrics.Types.PROXY_FAILURE); + res.status(404).send({ + error: { + code: 'unknown_provider_config', + message: + 'Provider config not found for the given provider config key. Please make sure the provider config exists in the Nango dashboard.' + } + }); + return; + } + + const connectionRes = await connectionService.getConnection(connectionId, providerConfigKey, environment.id); + if (connectionRes.error || !connectionRes.response) { + await logCtx.error('Failed to get connection', { error: connectionRes.error }); + await logCtx.failed(); + errorManager.errResFromNangoErr(res, connectionRes.error); + return; + } + + const credentialResponse = await connectionService.refreshOrTestCredentials({ account, environment, - connectionId, - providerConfigKey, + connection: connectionRes.response, + integration, logContextGetter, instantRefresh: false, onRefreshSuccess: connectionRefreshSuccessHook, @@ -99,24 +122,18 @@ class ProxyController { await logCtx.error('Failed to get connection credentials', { error: credentialResponse.error }); await logCtx.failed(); metrics.increment(metrics.Types.PROXY_FAILURE); - throw new Error(`Failed to get connection credentials: '${credentialResponse.error.message}'`); + res.status(400).send({ + error: { code: 'server_error', message: `Failed to get connection credentials: '${credentialResponse.error.message}'` } + }); + return; } const { value: connection } = credentialResponse; - const providerConfig = await configService.getProviderConfig(providerConfigKey, environment.id); - - if (!providerConfig) { - await logCtx.error('Provider configuration not found'); - await logCtx.failed(); - metrics.increment(metrics.Types.PROXY_FAILURE); - - throw new NangoError('unknown_provider_config'); - } await logCtx.enrichOperation({ - integrationId: providerConfig.id!, - integrationName: providerConfig.unique_key, - providerName: providerConfig.provider, + integrationId: integration.id!, + integrationName: integration.unique_key, + providerName: integration.provider, connectionId: connection.id!, connectionName: connection.connection_id }); @@ -124,7 +141,7 @@ class ProxyController { const internalConfig: InternalProxyConfiguration = { existingActivityLogId: logCtx.id, connection, - providerName: providerConfig.provider + providerName: integration.provider }; const { success, error, response: proxyConfig, logs } = proxyService.configure(externalConfig, internalConfig); @@ -143,6 +160,7 @@ class ProxyController { errorManager.errResFromNangoErr(res, error); await logCtx.failed(); metrics.increment(metrics.Types.PROXY_FAILURE); + res.status(400).send({ error: { code: 'server_error', message: 'failed to configure proxy' } }); return; } diff --git a/packages/server/lib/controllers/v1/connection/get.ts b/packages/server/lib/controllers/v1/connection/get.ts index 66dfc9af0e9..2663c67aac0 100644 --- a/packages/server/lib/controllers/v1/connection/get.ts +++ b/packages/server/lib/controllers/v1/connection/get.ts @@ -1,7 +1,7 @@ import { z } from 'zod'; import { asyncWrapper } from '../../../utils/asyncWrapper.js'; import { requireEmptyBody, zodErrorToHTTP } from '@nangohq/utils'; -import type { Connection, GetConnection, IntegrationConfig } from '@nangohq/types'; +import type { GetConnection, IntegrationConfig } from '@nangohq/types'; import { connectionService, configService, errorNotificationService } from '@nangohq/shared'; import { connectionRefreshFailed as connectionRefreshFailedHook, connectionRefreshSuccess as connectionRefreshSuccessHook } from '../../../hooks/hooks.js'; import { logContextGetter } from '@nangohq/logs'; @@ -53,11 +53,51 @@ export const getConnection = asyncWrapper(async (req, res) => { const instantRefresh = force_refresh === 'true'; const { connectionId } = params; - const credentialResponse = await connectionService.getConnectionCredentials({ + const integration: IntegrationConfig | null = await configService.getProviderConfig(providerConfigKey, environment.id); + if (!integration) { + res.status(404).send({ + error: { + code: 'unknown_provider_config', + message: 'Provider config not found for the given provider config key. Please make sure the provider config exists in the Nango dashboard.' + } + }); + return; + } + + const connectionRes = await connectionService.getConnection(connectionId, providerConfigKey, environment.id); + if (connectionRes.error || !connectionRes.response) { + switch (connectionRes.error?.type) { + case 'missing_connection': + res.status(400).send({ + error: { code: 'missing_connection', message: connectionRes.error.message } + }); + break; + case 'missing_provider_config': + res.status(400).send({ + error: { code: 'missing_provider_config', message: connectionRes.error.message } + }); + break; + case 'unknown_connection': + res.status(404).send({ + error: { code: 'unknown_connection', message: connectionRes.error.message } + }); + break; + case 'unknown_provider_config': + res.status(404).send({ + error: { code: 'unknown_provider_config', message: connectionRes.error.message } + }); + break; + default: + res.status(500).send({ error: { code: 'server_error' } }); + } + return; + } + + const credentialResponse = await connectionService.refreshOrTestCredentials({ account, environment, - connectionId, - providerConfigKey, + connection: connectionRes.response, + integration, logContextGetter, instantRefresh, onRefreshSuccess: connectionRefreshSuccessHook, @@ -65,75 +105,25 @@ export const getConnection = asyncWrapper(async (req, res) => { }); if (credentialResponse.isErr()) { - if (credentialResponse.error.payload && credentialResponse.error.payload['id']) { - const errorConnection = credentialResponse.error.payload as unknown as Connection; - const errorLog = await errorNotificationService.auth.get(errorConnection.id as number); - - res.status(400).send({ - errorLog, - provider: null, // TODO: fix this - connection: errorConnection - }); - } else { - switch (credentialResponse.error.type) { - case 'missing_connection': - res.status(400).send({ - error: { - code: 'missing_connection', - message: credentialResponse.error.message - } - }); - break; - case 'missing_provider_config': - res.status(400).send({ - error: { - code: 'missing_provider_config', - message: credentialResponse.error.message - } - }); - break; - case 'unknown_connection': - res.status(404).send({ - error: { - code: 'unknown_connection', - message: credentialResponse.error.message - } - }); - break; - case 'unknown_provider_config': - res.status(404).send({ - error: { - code: 'unknown_provider_config', - message: credentialResponse.error.message - } - }); - break; - } - } - return; - } - - const { value: connection } = credentialResponse; + const errorLog = await errorNotificationService.auth.get(connectionRes.response.id as number); - const config: IntegrationConfig | null = await configService.getProviderConfig(connection.provider_config_key, environment.id); + // When we failed to refresh we still return a 200 because the connection is used in the UI + // Ultimately this could be a second endpoint so the UI displays faster and no confusion between error code + res.status(200).send({ errorLog, provider: integration.provider, connection: connectionRes.response }); - if (!config) { - res.status(404).send({ - error: { - code: 'unknown_provider_config', - message: 'Provider config not found for the given provider config key. Please make sure the provider config exists in the Nango dashboard.' - } - }); return; } + const connection = credentialResponse.value; + if (instantRefresh) { + // If we force the refresh we also specifically log a success operation (we usually only log error) const logCtx = await logContextGetter.create( { operation: { type: 'auth', action: 'refresh_token' } }, { account, environment, - integration: { id: config.id!, name: config.unique_key, provider: config.provider }, + integration: { id: integration.id!, name: integration.unique_key, provider: integration.provider }, connection: { id: connection.id!, name: connection.connection_id } } ); @@ -142,7 +132,7 @@ export const getConnection = asyncWrapper(async (req, res) => { } res.status(200).send({ - provider: config.provider, + provider: integration.provider, connection, errorLog: null }); diff --git a/packages/server/lib/hooks/connection/post-connection.ts b/packages/server/lib/hooks/connection/post-connection.ts index 83255112bb6..9dcea62afb8 100644 --- a/packages/server/lib/hooks/connection/post-connection.ts +++ b/packages/server/lib/hooks/connection/post-connection.ts @@ -4,7 +4,6 @@ import { LogActionEnum, LogTypes, proxyService, connectionService, telemetry, ge import * as postConnectionHandlers from './index.js'; import type { LogContext, LogContextGetter } from '@nangohq/logs'; import { stringifyError } from '@nangohq/utils'; -import { connectionRefreshFailed as connectionRefreshFailedHook, connectionRefreshSuccess as connectionRefreshSuccessHook } from '../hooks.js'; import type { InternalProxyConfiguration } from '@nangohq/types'; type PostConnectionHandler = (internalNango: InternalNango) => Promise; @@ -23,22 +22,12 @@ async function execute(createdConnection: RecentlyCreatedConnection, providerNam const { connection: upsertedConnection, environment, account } = createdConnection; let logCtx: LogContext | undefined; try { - const credentialResponse = await connectionService.getConnectionCredentials({ - account, - environment, - connectionId: upsertedConnection.connection_id, - providerConfigKey: upsertedConnection.provider_config_key, - logContextGetter, - instantRefresh: false, - onRefreshSuccess: connectionRefreshSuccessHook, - onRefreshFailed: connectionRefreshFailedHook - }); - - if (credentialResponse.isErr()) { + const connectionRes = await connectionService.getConnection(upsertedConnection.connection_id, upsertedConnection.provider_config_key, environment.id); + if (connectionRes.error || !connectionRes.response) { return; } - const { value: connection } = credentialResponse; + const connection = connectionRes.response; const internalConfig: InternalProxyConfiguration = { connection, diff --git a/packages/server/lib/refreshConnections.ts b/packages/server/lib/refreshConnections.ts index 9ff9e19c427..68a3470ef30 100644 --- a/packages/server/lib/refreshConnections.ts +++ b/packages/server/lib/refreshConnections.ts @@ -36,7 +36,7 @@ export function refreshConnectionsCron(): void { } export async function exec(): Promise { - return await tracer.trace>('nango.server.cron.refreshConnections', async (span) => { + await tracer.trace>('nango.server.cron.refreshConnections', async (span) => { let lock: Lock | undefined; try { logger.info(`${cronName} starting`); @@ -56,21 +56,22 @@ export async function exec(): Promise { const limit = 1000; // eslint-disable-next-line no-constant-condition while (true) { - const staleConnections = await connectionService.getStaleConnections({ days: 0, limit, cursor }); + const staleConnections = await connectionService.getStaleConnections({ days: 1, limit, cursor }); logger.info(`${cronName} found ${staleConnections.length} stale connections`); for (const staleConnection of staleConnections) { if (Date.now() - startTimestamp > ttlMs) { logger.info(`${cronName} time limit reached, stopping`); return; } - const { connection_id, environment, provider_config_key, account } = staleConnection; - logger.info(`${cronName} refreshing connection '${connection_id}' for accountId '${account.id}'`); + + const { connection, account, environment, integration } = staleConnection; + logger.info(`${cronName} refreshing connection '${connection.connection_id}' for accountId '${account.id}'`); try { - const credentialResponse = await connectionService.getConnectionCredentials({ + const credentialResponse = await connectionService.refreshOrTestCredentials({ account, environment, - connectionId: connection_id, - providerConfigKey: provider_config_key, + integration, + connection, logContextGetter, instantRefresh: false, onRefreshSuccess: connectionRefreshSuccessHook, @@ -83,7 +84,7 @@ export async function exec(): Promise { metrics.increment(metrics.Types.REFRESH_CONNECTIONS_FAILED); } } catch (err) { - logger.error(`${cronName} failed to refresh connection '${connection_id}' ${stringifyError(err)}`); + logger.error(`${cronName} failed to refresh connection '${connection.connection_id}' ${stringifyError(err)}`); metrics.increment(metrics.Types.REFRESH_CONNECTIONS_FAILED); } cursor = staleConnection.cursor; diff --git a/packages/shared/lib/services/connection.service.ts b/packages/shared/lib/services/connection.service.ts index 9dead17c623..f44af625fe9 100644 --- a/packages/shared/lib/services/connection.service.ts +++ b/packages/shared/lib/services/connection.service.ts @@ -24,7 +24,8 @@ import type { DBTeam, DBEnvironment, JwtCredentials, - BillCredentials + BillCredentials, + IntegrationConfig } from '@nangohq/types'; import { getLogger, stringifyError, Ok, Err, axiosInstance as axios } from '@nangohq/utils'; import type { Result } from '@nangohq/utils'; @@ -776,7 +777,7 @@ class ConnectionService { days: number; limit: number; cursor?: number | undefined; - }): Promise<{ connection_id: string; provider_config_key: string; account: DBTeam; environment: DBEnvironment; cursor: number }[]> { + }): Promise<{ connection: Connection; account: DBTeam; environment: DBEnvironment; cursor: number; integration: ProviderConfig }[]> { const dateThreshold = new Date(); dateThreshold.setDate(dateThreshold.getDate() - days); @@ -788,11 +789,10 @@ class ConnectionService { .join('_nango_environments', '_nango_connections.environment_id', '_nango_environments.id') .join('_nango_accounts', '_nango_environments.account_id', '_nango_accounts.id') .select( - '_nango_connections.connection_id as connection_id', - '_nango_connections.provider_config_key as provider_config_key', + db.knex.raw('row_to_json(_nango_connections.*) as connection'), + db.knex.raw('row_to_json(_nango_configs.*) as integration'), db.knex.raw('row_to_json(_nango_environments.*) as environment'), - db.knex.raw('row_to_json(_nango_accounts.*) as account'), - '_nango_connections.id as cursor' + db.knex.raw('row_to_json(_nango_accounts.*) as account') ) .where('_nango_connections.deleted', false) .andWhere((builder) => builder.where('last_fetched_at', '<', dateThreshold).orWhereNull('last_fetched_at')) @@ -946,11 +946,11 @@ class ConnectionService { return del; } - public async getConnectionCredentials({ + public async refreshOrTestCredentials({ account, environment, - connectionId, - providerConfigKey, + connection, + integration, logContextGetter, instantRefresh, onRefreshSuccess, @@ -959,8 +959,8 @@ class ConnectionService { }: { account: DBTeam; environment: DBEnvironment; - connectionId: string; - providerConfigKey: string; + connection: Connection; + integration: IntegrationConfig; logContextGetter: LogContextGetter; instantRefresh: boolean; onRefreshSuccess: (args: { connection: Connection; environment: DBEnvironment; config: ProviderConfig }) => Promise; @@ -985,55 +985,26 @@ class ConnectionService { ) => Promise>) | undefined; }): Promise> { - if (connectionId === null) { - const error = new NangoError('missing_connection'); - - return Err(error); - } - - if (providerConfigKey === null) { - const error = new NangoError('missing_provider_config'); - - return Err(error); - } - - const { success, error, response: connection } = await this.getConnection(connectionId, providerConfigKey, environment.id); - - if (!success && error) { - return Err(error); - } - - if (connection === null || !connection.id) { - const error = new NangoError('unknown_connection', { connectionId, providerConfigKey, environmentName: environment.name }); - - return Err(error); - } - - const config: ProviderConfig | null = await configService.getProviderConfig(connection?.provider_config_key, environment.id); - - if (config === null || !config.id) { - const error = new NangoError('unknown_provider_config'); - return Err(error); - } - - const provider = getProvider(config?.provider); + const provider = getProvider(integration.provider); if (!provider) { const error = new NangoError('unknown_provider_config'); return Err(error); } + const copy = { ...connection }; + if ( - connection?.credentials?.type === 'OAUTH2' || - connection?.credentials?.type === 'APP' || - connection?.credentials?.type === 'OAUTH2_CC' || - connection?.credentials?.type === 'TABLEAU' || - connection?.credentials?.type === 'JWT' || - connection?.credentials?.type === 'BILL' + connection.credentials?.type === 'OAUTH2' || + connection.credentials?.type === 'APP' || + connection.credentials?.type === 'OAUTH2_CC' || + connection.credentials?.type === 'TABLEAU' || + connection.credentials?.type === 'JWT' || + connection.credentials?.type === 'BILL' ) { const { success, error, response } = await this.refreshCredentialsIfNeeded({ connectionId: connection.connection_id, environmentId: environment.id, - providerConfig: config, + providerConfig: integration, provider: provider as ProviderOAuth2, environment_id: environment.id, instantRefresh @@ -1045,8 +1016,8 @@ class ConnectionService { { account, environment, - integration: config ? { id: config.id, name: config.unique_key, provider: config.provider } : undefined, - connection: { id: connection.id, name: connection.connection_id } + integration: integration ? { id: integration.id!, name: integration.unique_key, provider: integration.provider } : undefined, + connection: { id: connection.id!, name: connection.connection_id } } ); @@ -1063,36 +1034,36 @@ class ConnectionService { }, environment, provider, - config, + config: integration, action: 'token_refresh' }); } - const { credentials, ...connectionWithoutCredentials } = connection; - const errorWithPayload = new NangoError(error!.type, connectionWithoutCredentials); + const { credentials, ...connectionWithoutCredentials } = copy; + const errorWithPayload = new NangoError(error!.type, { connection: connectionWithoutCredentials }); // there was an attempt to refresh the token so clear it from the queue // of connections to refresh if it failed - await this.updateLastFetched(connection.id); + await this.updateLastFetched(connection.id!); return Err(errorWithPayload); } else if (response.refreshed) { await onRefreshSuccess({ connection, environment, - config + config: integration }); } - connection.credentials = response.credentials as OAuth2Credentials; - } else if (connection?.credentials?.type === 'BASIC' || connection?.credentials?.type === 'API_KEY' || connection?.credentials?.type === 'TBA') { + copy.credentials = response.credentials as OAuth2Credentials; + } else if (connection.credentials?.type === 'BASIC' || connection.credentials?.type === 'API_KEY' || connection.credentials?.type === 'TBA') { if (connectionTestHook) { const result = await connectionTestHook( - config.provider, + integration.provider, provider, connection.credentials, connection.connection_id, - providerConfigKey, + integration.unique_key, environment.id, connection.connection_config ); @@ -1102,8 +1073,8 @@ class ConnectionService { { account, environment, - integration: config ? { id: config.id, name: config.unique_key, provider: config.provider } : undefined, - connection: { id: connection.id, name: connection.connection_id } + integration: integration ? { id: integration.id!, name: integration.unique_key, provider: integration.provider } : undefined, + connection: { id: connection.id!, name: connection.connection_id } } ); @@ -1118,30 +1089,30 @@ class ConnectionService { }, environment, provider, - config, + config: integration, action: 'connection_test' }); // there was an attempt to test the credentials // so clear it from the queue if it failed - await this.updateLastFetched(connection.id); + await this.updateLastFetched(connection.id!); - const { credentials, ...connectionWithoutCredentials } = connection; + const { credentials, ...connectionWithoutCredentials } = copy; const errorWithPayload = new NangoError(result.error.type, connectionWithoutCredentials); return Err(errorWithPayload); } else { await onRefreshSuccess({ connection, environment, - config + config: integration }); } } } - await this.updateLastFetched(connection.id); + await this.updateLastFetched(connection.id!); - return Ok(connection); + return Ok(copy); } public async updateLastFetched(id: number) {