From e37920b4f20c485a2ad8413910033d18045b7f61 Mon Sep 17 00:00:00 2001 From: Gaurav Kochar Date: Mon, 1 Jul 2024 14:48:37 +0530 Subject: [PATCH 01/10] resolved conflicts --- packages/core/src/destination-kit/index.ts | 114 ++++++++++++++++-- .../src/destinations/amazon-amc/index.ts | 43 ++++--- 2 files changed, 130 insertions(+), 27 deletions(-) diff --git a/packages/core/src/destination-kit/index.ts b/packages/core/src/destination-kit/index.ts index f794968524..fc24ec734e 100644 --- a/packages/core/src/destination-kit/index.ts +++ b/packages/core/src/destination-kit/index.ts @@ -34,6 +34,7 @@ import { AuthTokens, getAuthData, getOAuth2Data, updateOAuthSettings } from './p import { InputData, Features } from '../mapping-kit' import { retry } from '../retry' import { HTTPError } from '..' +import isEmpty from 'lodash/isEmpty' export type { BaseActionDefinition, @@ -95,6 +96,12 @@ export type AudienceResult = { export type AudienceMode = { type: 'realtime' } | { type: 'synced'; full_audience_sync: boolean } +// interface OnEventOptions { +// onTokenRefresh?: (tokens: RefreshAccessTokenResult) => Promise +// /** Handler to perform synchronization. If set, the refresh access token method will be synchronized across +// * all events across multiple instances of the destination using the same account for a given source*/ +// synchronizeRefreshAccessToken?: () => Promise +// } export type CreateAudienceInput = { settings: Settings @@ -102,6 +109,8 @@ export type CreateAudienceInput audienceName: string + options?: OnEventOptions + statsContext?: StatsContext } @@ -112,6 +121,8 @@ export type GetAudienceInput = { externalId: string + options?: OnEventOptions + statsContext?: StatsContext } @@ -140,9 +151,8 @@ const instanceOfAudienceDestinationSettingsWithCreateGet = ( export interface AudienceDestinationDefinition extends DestinationDefinition { - audienceConfig: - | AudienceDestinationConfiguration - | AudienceDestinationConfigurationWithCreateGet + audienceConfig: AudienceDestinationConfigurationWithCreateGet + // | AudienceDestinationConfiguration audienceFields: Record @@ -422,41 +432,119 @@ export class Destination { } async createAudience(createAudienceInput: CreateAudienceInput) { + let settings: JSONObject = createAudienceInput.settings as unknown as JSONObject + const options: OnEventOptions | undefined = createAudienceInput.options const audienceDefinition = this.definition as AudienceDestinationDefinition if (!instanceOfAudienceDestinationSettingsWithCreateGet(audienceDefinition.audienceConfig)) { throw new Error('Unexpected call to createAudience') } - const destinationSettings = this.getDestinationSettings(createAudienceInput.settings as unknown as JSONObject) - const auth = getAuthData(createAudienceInput.settings as unknown as JSONObject) + //validate audienceField Input + if (!isEmpty(createAudienceInput.audienceSettings)) { + validateSchema(createAudienceInput.audienceSettings, fieldsToJsonSchema(audienceDefinition.audienceFields)) + } + // const destinationSettings = this.getDestinationSettings(createAudienceInput.settings as unknown as JSONObject) + // const auth = getAuthData(createAudienceInput.settings as unknown as JSONObject) + const destinationSettings = this.getDestinationSettings(settings) + const auth = getAuthData(settings) const context: ExecuteInput = { audienceSettings: createAudienceInput.audienceSettings, settings: destinationSettings, payload: undefined, auth } - const options = this.extendRequest?.(context) ?? {} - const requestClient = createRequestClient({ ...options, statsContext: context.statsContext }) + const opts = this.extendRequest?.(context) ?? {} + const requestClient = createRequestClient({ ...opts, statsContext: context.statsContext }) + + const run = async () => { + return await audienceDefinition.audienceConfig?.createAudience(requestClient, createAudienceInput) + } + + const onFailedAttempt = async (error: ResponseError & HTTPError) => { + const statusCode = error?.status ?? error?.response?.status ?? 500 + + // Throw original error if it is unrelated to invalid access tokens and not an oauth2 scheme + if ( + !( + statusCode === 401 && + (this.authentication?.scheme === 'oauth2' || this.authentication?.scheme === 'oauth-managed') + ) + ) { + throw error + } + + const oauthSettings = getOAuth2Data(settings) + const newTokens = await this.refreshAccessToken( + destinationSettings, + oauthSettings, + options?.synchronizeRefreshAccessToken + ) + if (!newTokens) { + throw new InvalidAuthenticationError('Failed to refresh access token', ErrorCodes.OAUTH_REFRESH_FAILED) + } + + // Update `settings` with new tokens + settings = updateOAuthSettings(settings, newTokens) + console.log('COMING HERE 2', newTokens, settings) + + await options?.onTokenRefresh?.(newTokens) + } - return audienceDefinition.audienceConfig?.createAudience(requestClient, createAudienceInput) + return await retry(run, { retries: 2, onFailedAttempt }) + // return audienceDefinition.audienceConfig?.createAudience(requestClient, createAudienceInput) } async getAudience(getAudienceInput: GetAudienceInput) { const audienceDefinition = this.definition as AudienceDestinationDefinition + let settings: JSONObject = getAudienceInput.settings as unknown as JSONObject + const options: OnEventOptions | undefined = getAudienceInput.options if (!instanceOfAudienceDestinationSettingsWithCreateGet(audienceDefinition.audienceConfig)) { throw new Error('Unexpected call to getAudience') } - const destinationSettings = this.getDestinationSettings(getAudienceInput.settings as unknown as JSONObject) - const auth = getAuthData(getAudienceInput.settings as unknown as JSONObject) + const destinationSettings = this.getDestinationSettings(settings) + const auth = getAuthData(settings) const context: ExecuteInput = { audienceSettings: getAudienceInput.audienceSettings, settings: destinationSettings, payload: undefined, auth } - const options = this.extendRequest?.(context) ?? {} - const requestClient = createRequestClient({ ...options, statsContext: context.statsContext }) + const opts = this.extendRequest?.(context) ?? {} + const requestClient = createRequestClient({ ...opts, statsContext: context.statsContext }) + const run = async () => { + return audienceDefinition.audienceConfig?.getAudience(requestClient, getAudienceInput) + } + + const onFailedAttempt = async (error: ResponseError & HTTPError) => { + const statusCode = error?.status ?? error?.response?.status ?? 500 + console.log('COMING HERE 1', statusCode) - return audienceDefinition.audienceConfig?.getAudience(requestClient, getAudienceInput) + // Throw original error if it is unrelated to invalid access tokens and not an oauth2 scheme + if ( + !( + statusCode === 401 && + (this.authentication?.scheme === 'oauth2' || this.authentication?.scheme === 'oauth-managed') + ) + ) { + throw error + } + + const oauthSettings = getOAuth2Data(settings) + const newTokens = await this.refreshAccessToken( + destinationSettings, + oauthSettings, + options?.synchronizeRefreshAccessToken + ) + if (!newTokens) { + throw new InvalidAuthenticationError('Failed to refresh access token', ErrorCodes.OAUTH_REFRESH_FAILED) + } + + // Update `settings` with new tokens + settings = updateOAuthSettings(settings, newTokens) + + await options?.onTokenRefresh?.(newTokens) + } + + return await retry(run, { retries: 2, onFailedAttempt }) } async testAuthentication(settings: Settings): Promise { diff --git a/packages/destination-actions/src/destinations/amazon-amc/index.ts b/packages/destination-actions/src/destinations/amazon-amc/index.ts index ff0244a582..7ca406f6ea 100644 --- a/packages/destination-actions/src/destinations/amazon-amc/index.ts +++ b/packages/destination-actions/src/destinations/amazon-amc/index.ts @@ -4,7 +4,7 @@ import type { Settings, AudienceSettings } from './generated-types' import { AudiencePayload, extractNumberAndSubstituteWithStringValue, - getAuthSettings, + // getAuthSettings, getAuthToken, REGEX_ADVERTISERID, REGEX_AUDIENCEID @@ -137,7 +137,9 @@ const destination: AudienceDestinationDefinition = { full_audience_sync: false // If true, we send the entire audience. If false, we just send the delta. }, async createAudience(request, createAudienceInput) { - const { audienceName, audienceSettings, settings } = createAudienceInput + // console.log('createAudience>>>>>>>>>>>>>>>') + const { audienceName, audienceSettings, settings, statsContext } = createAudienceInput + const { statsClient, tags: statsTags } = statsContext || {} const endpoint = settings.region const description = audienceSettings?.description const advertiser_id = audienceSettings?.advertiserId @@ -147,6 +149,10 @@ const destination: AudienceDestinationDefinition = { const currency = audienceSettings?.currency const cpm_cents = audienceSettings?.cpmCents + const statsName = 'createAmazonAudience' + statsTags?.push(`slug:${destination.slug}`) + statsClient?.incr(`${statsName}.intialise`, 1, statsTags) + if (!advertiser_id) { throw new IntegrationError('Missing advertiserId Value', 'MISSING_REQUIRED_FIELD', 400) } @@ -199,8 +205,11 @@ const destination: AudienceDestinationDefinition = { } // @ts-ignore - TS doesn't know about the oauth property - const authSettings = getAuthSettings(settings) - const authToken = await getAuthToken(request, createAudienceInput.settings, authSettings) + // const authSettings = getAuthSettings(settings) + // console.log('createAudience>>>>>>>>>>>>>>>2', authSettings) + + // const authToken = await getAuthToken(request, createAudienceInput.settings, authSettings) + // console.log('createAudience>>>>>>>>>>>>>>>3', authSettings) let payloadString = JSON.stringify(payload) // Regular expression to find a advertiserId numeric string and replace the quoted advertiserId string with an unquoted number @@ -211,14 +220,16 @@ const destination: AudienceDestinationDefinition = { method: 'POST', body: payloadString, headers: { - 'Content-Type': 'application/vnd.amcaudiences.v1+json', - authorization: `Bearer ${authToken}` + 'Content-Type': 'application/vnd.amcaudiences.v1+json' + // authorization: `Bearer ${authToken}` } }) const res = await response.text() // Regular expression to find a audienceId number and replace the audienceId with quoted string const resp = extractNumberAndSubstituteWithStringValue(res, REGEX_AUDIENCEID, '"audienceId":"$1"') + statsClient?.incr(`${statsName}.sucsess`, 1, statsTags) + return { externalId: resp.audienceId } @@ -227,24 +238,28 @@ const destination: AudienceDestinationDefinition = { async getAudience(request, getAudienceInput) { // getAudienceInput.externalId represents audience ID that was created in createAudience const audience_id = getAudienceInput.externalId - const { settings } = getAudienceInput + const { settings, statsContext } = getAudienceInput const endpoint = settings.region - + const { statsClient, tags: statsTags } = statsContext || {} if (!audience_id) { throw new IntegrationError('Missing audienceId value', 'MISSING_REQUIRED_FIELD', 400) } // @ts-ignore - TS doesn't know about the oauth property - const authSettings = getAuthSettings(settings) - const authToken = await getAuthToken(request, settings, authSettings) + // const authSettings = getAuthSettings(settings) + // const authToken = await getAuthToken(request, settings, authSettings) + const statsName = 'getAmazonAudience' + statsTags?.push(`slug:${destination.slug}`) + statsClient?.incr(`${statsName}.intialise`, 1, statsTags) const response = await request(`${endpoint}/amc/audiences/metadata/${audience_id}`, { - method: 'GET', - headers: { - authorization: `Bearer ${authToken}` - } + method: 'GET' + // headers: { + // authorization: `Bearer ${authToken}` + // } }) const res = await response.text() // Regular expression to find a audienceId number and replace the audienceId with quoted string const resp = extractNumberAndSubstituteWithStringValue(res, REGEX_AUDIENCEID, '"audienceId":"$1"') + statsClient?.incr(`${statsName}.success`, 1, statsTags) return { externalId: resp.audienceId } From 8abafda9caab62e7ef335d05ec756ac10e4a8b85 Mon Sep 17 00:00:00 2001 From: Gaurav Kochar Date: Tue, 2 Jul 2024 16:46:45 +0530 Subject: [PATCH 02/10] Worked on reauthentication flow for createAudience and getAudience --- packages/core/src/destination-kit/index.ts | 42 ++++++++----------- .../amazon-amc/__tests__/index.test.ts | 28 ++++++------- .../src/destinations/amazon-amc/index.ts | 3 -- 3 files changed, 32 insertions(+), 41 deletions(-) diff --git a/packages/core/src/destination-kit/index.ts b/packages/core/src/destination-kit/index.ts index fc24ec734e..8566616003 100644 --- a/packages/core/src/destination-kit/index.ts +++ b/packages/core/src/destination-kit/index.ts @@ -442,20 +442,17 @@ export class Destination { if (!isEmpty(createAudienceInput.audienceSettings)) { validateSchema(createAudienceInput.audienceSettings, fieldsToJsonSchema(audienceDefinition.audienceFields)) } - // const destinationSettings = this.getDestinationSettings(createAudienceInput.settings as unknown as JSONObject) - // const auth = getAuthData(createAudienceInput.settings as unknown as JSONObject) const destinationSettings = this.getDestinationSettings(settings) - const auth = getAuthData(settings) - const context: ExecuteInput = { - audienceSettings: createAudienceInput.audienceSettings, - settings: destinationSettings, - payload: undefined, - auth - } - const opts = this.extendRequest?.(context) ?? {} - const requestClient = createRequestClient({ ...opts, statsContext: context.statsContext }) - const run = async () => { + const auth = getAuthData(settings) + const context: ExecuteInput = { + audienceSettings: createAudienceInput.audienceSettings, + settings: destinationSettings, + payload: undefined, + auth + } + const opts = this.extendRequest?.(context) ?? {} + const requestClient = createRequestClient({ ...opts, statsContext: context.statsContext }) return await audienceDefinition.audienceConfig?.createAudience(requestClient, createAudienceInput) } @@ -484,8 +481,6 @@ export class Destination { // Update `settings` with new tokens settings = updateOAuthSettings(settings, newTokens) - console.log('COMING HERE 2', newTokens, settings) - await options?.onTokenRefresh?.(newTokens) } @@ -501,22 +496,21 @@ export class Destination { throw new Error('Unexpected call to getAudience') } const destinationSettings = this.getDestinationSettings(settings) - const auth = getAuthData(settings) - const context: ExecuteInput = { - audienceSettings: getAudienceInput.audienceSettings, - settings: destinationSettings, - payload: undefined, - auth - } - const opts = this.extendRequest?.(context) ?? {} - const requestClient = createRequestClient({ ...opts, statsContext: context.statsContext }) const run = async () => { + const auth = getAuthData(settings) + const context: ExecuteInput = { + audienceSettings: getAudienceInput.audienceSettings, + settings: destinationSettings, + payload: undefined, + auth + } + const opts = this.extendRequest?.(context) ?? {} + const requestClient = createRequestClient({ ...opts, statsContext: context.statsContext }) return audienceDefinition.audienceConfig?.getAudience(requestClient, getAudienceInput) } const onFailedAttempt = async (error: ResponseError & HTTPError) => { const statusCode = error?.status ?? error?.response?.status ?? 500 - console.log('COMING HERE 1', statusCode) // Throw original error if it is unrelated to invalid access tokens and not an oauth2 scheme if ( diff --git a/packages/destination-actions/src/destinations/amazon-amc/__tests__/index.test.ts b/packages/destination-actions/src/destinations/amazon-amc/__tests__/index.test.ts index 2498008c66..0ff6d01a42 100644 --- a/packages/destination-actions/src/destinations/amazon-amc/__tests__/index.test.ts +++ b/packages/destination-actions/src/destinations/amazon-amc/__tests__/index.test.ts @@ -1,5 +1,5 @@ import nock from 'nock' -import { createTestIntegration, InvalidAuthenticationError } from '@segment/actions-core' +import { createTestIntegration } from '@segment/actions-core' import Definition from '../index' import { HTTPError } from '@segment/actions-core/*' import { AUTHORIZATION_URL } from '../utils' @@ -116,14 +116,14 @@ describe('Amazon-Ads (actions)', () => { ) }) - it('should fail if refresh token API gets failed', async () => { - const endpoint = AUTHORIZATION_URL[`${settings.region}`] - nock(`${endpoint}`).post('/auth/o2/token').reply(401) + // it('should fail if refresh token API gets failed', async () => { + // const endpoint = AUTHORIZATION_URL[`${settings.region}`] + // nock(`${endpoint}`).post('/auth/o2/token').reply(401) - await expect(testDestination.createAudience(createAudienceInputTemp)).rejects.toThrowError( - InvalidAuthenticationError - ) - }) + // await expect(testDestination.createAudience(createAudienceInputTemp)).rejects.toThrowError( + // InvalidAuthenticationError + // ) + // }) it('should throw an HTTPError when createAudience API response is not ok', async () => { const endpoint = AUTHORIZATION_URL[`${settings.region}`] @@ -205,13 +205,13 @@ describe('Amazon-Ads (actions)', () => { await expect(audiencePromise).rejects.toHaveProperty('response.statusText', 'Not Found') await expect(audiencePromise).rejects.toHaveProperty('response.status', 404) }) - it('should fail if refresh token API gets failed ', async () => { - const endpoint = AUTHORIZATION_URL[`${settings.region}`] - nock(`${endpoint}`).post('/auth/o2/token').reply(401) + // it('should fail if refresh token API gets failed ', async () => { + // const endpoint = AUTHORIZATION_URL[`${settings.region}`] + // nock(`${endpoint}`).post('/auth/o2/token').reply(401) - const audiencePromise = testDestination.getAudience(getAudienceInput) - await expect(audiencePromise).rejects.toThrow(InvalidAuthenticationError) - }) + // const audiencePromise = testDestination.getAudience(getAudienceInput) + // await expect(audiencePromise).rejects.toThrow(InvalidAuthenticationError) + // }) it('should throw an IntegrationError when the audienceId is not provided', async () => { getAudienceInput.externalId = '' diff --git a/packages/destination-actions/src/destinations/amazon-amc/index.ts b/packages/destination-actions/src/destinations/amazon-amc/index.ts index 7ca406f6ea..946502bea7 100644 --- a/packages/destination-actions/src/destinations/amazon-amc/index.ts +++ b/packages/destination-actions/src/destinations/amazon-amc/index.ts @@ -137,7 +137,6 @@ const destination: AudienceDestinationDefinition = { full_audience_sync: false // If true, we send the entire audience. If false, we just send the delta. }, async createAudience(request, createAudienceInput) { - // console.log('createAudience>>>>>>>>>>>>>>>') const { audienceName, audienceSettings, settings, statsContext } = createAudienceInput const { statsClient, tags: statsTags } = statsContext || {} const endpoint = settings.region @@ -206,10 +205,8 @@ const destination: AudienceDestinationDefinition = { // @ts-ignore - TS doesn't know about the oauth property // const authSettings = getAuthSettings(settings) - // console.log('createAudience>>>>>>>>>>>>>>>2', authSettings) // const authToken = await getAuthToken(request, createAudienceInput.settings, authSettings) - // console.log('createAudience>>>>>>>>>>>>>>>3', authSettings) let payloadString = JSON.stringify(payload) // Regular expression to find a advertiserId numeric string and replace the quoted advertiserId string with an unquoted number From 32a9205eabc86fc6925a2ad02d021864b9cfe6f6 Mon Sep 17 00:00:00 2001 From: Gaurav Kochar Date: Thu, 4 Jul 2024 13:30:10 +0530 Subject: [PATCH 03/10] Removed token refresh and Stats from amazon-amc --- packages/core/src/destination-kit/index.ts | 34 ++----------------- .../src/destinations/amazon-amc/index.ts | 28 ++------------- 2 files changed, 5 insertions(+), 57 deletions(-) diff --git a/packages/core/src/destination-kit/index.ts b/packages/core/src/destination-kit/index.ts index 8566616003..29f03da02d 100644 --- a/packages/core/src/destination-kit/index.ts +++ b/packages/core/src/destination-kit/index.ts @@ -34,7 +34,6 @@ import { AuthTokens, getAuthData, getOAuth2Data, updateOAuthSettings } from './p import { InputData, Features } from '../mapping-kit' import { retry } from '../retry' import { HTTPError } from '..' -import isEmpty from 'lodash/isEmpty' export type { BaseActionDefinition, @@ -96,12 +95,6 @@ export type AudienceResult = { export type AudienceMode = { type: 'realtime' } | { type: 'synced'; full_audience_sync: boolean } -// interface OnEventOptions { -// onTokenRefresh?: (tokens: RefreshAccessTokenResult) => Promise -// /** Handler to perform synchronization. If set, the refresh access token method will be synchronized across -// * all events across multiple instances of the destination using the same account for a given source*/ -// synchronizeRefreshAccessToken?: () => Promise -// } export type CreateAudienceInput = { settings: Settings @@ -109,8 +102,6 @@ export type CreateAudienceInput audienceName: string - options?: OnEventOptions - statsContext?: StatsContext } @@ -121,8 +112,6 @@ export type GetAudienceInput = { externalId: string - options?: OnEventOptions - statsContext?: StatsContext } @@ -433,15 +422,10 @@ export class Destination { async createAudience(createAudienceInput: CreateAudienceInput) { let settings: JSONObject = createAudienceInput.settings as unknown as JSONObject - const options: OnEventOptions | undefined = createAudienceInput.options const audienceDefinition = this.definition as AudienceDestinationDefinition if (!instanceOfAudienceDestinationSettingsWithCreateGet(audienceDefinition.audienceConfig)) { throw new Error('Unexpected call to createAudience') } - //validate audienceField Input - if (!isEmpty(createAudienceInput.audienceSettings)) { - validateSchema(createAudienceInput.audienceSettings, fieldsToJsonSchema(audienceDefinition.audienceFields)) - } const destinationSettings = this.getDestinationSettings(settings) const run = async () => { const auth = getAuthData(settings) @@ -470,28 +454,20 @@ export class Destination { } const oauthSettings = getOAuth2Data(settings) - const newTokens = await this.refreshAccessToken( - destinationSettings, - oauthSettings, - options?.synchronizeRefreshAccessToken - ) + const newTokens = await this.refreshAccessToken(destinationSettings, oauthSettings) if (!newTokens) { throw new InvalidAuthenticationError('Failed to refresh access token', ErrorCodes.OAUTH_REFRESH_FAILED) } // Update `settings` with new tokens settings = updateOAuthSettings(settings, newTokens) - await options?.onTokenRefresh?.(newTokens) } - return await retry(run, { retries: 2, onFailedAttempt }) - // return audienceDefinition.audienceConfig?.createAudience(requestClient, createAudienceInput) } async getAudience(getAudienceInput: GetAudienceInput) { const audienceDefinition = this.definition as AudienceDestinationDefinition let settings: JSONObject = getAudienceInput.settings as unknown as JSONObject - const options: OnEventOptions | undefined = getAudienceInput.options if (!instanceOfAudienceDestinationSettingsWithCreateGet(audienceDefinition.audienceConfig)) { throw new Error('Unexpected call to getAudience') } @@ -523,19 +499,13 @@ export class Destination { } const oauthSettings = getOAuth2Data(settings) - const newTokens = await this.refreshAccessToken( - destinationSettings, - oauthSettings, - options?.synchronizeRefreshAccessToken - ) + const newTokens = await this.refreshAccessToken(destinationSettings, oauthSettings) if (!newTokens) { throw new InvalidAuthenticationError('Failed to refresh access token', ErrorCodes.OAUTH_REFRESH_FAILED) } // Update `settings` with new tokens settings = updateOAuthSettings(settings, newTokens) - - await options?.onTokenRefresh?.(newTokens) } return await retry(run, { retries: 2, onFailedAttempt }) diff --git a/packages/destination-actions/src/destinations/amazon-amc/index.ts b/packages/destination-actions/src/destinations/amazon-amc/index.ts index 946502bea7..c8287d2a97 100644 --- a/packages/destination-actions/src/destinations/amazon-amc/index.ts +++ b/packages/destination-actions/src/destinations/amazon-amc/index.ts @@ -137,8 +137,7 @@ const destination: AudienceDestinationDefinition = { full_audience_sync: false // If true, we send the entire audience. If false, we just send the delta. }, async createAudience(request, createAudienceInput) { - const { audienceName, audienceSettings, settings, statsContext } = createAudienceInput - const { statsClient, tags: statsTags } = statsContext || {} + const { audienceName, audienceSettings, settings } = createAudienceInput const endpoint = settings.region const description = audienceSettings?.description const advertiser_id = audienceSettings?.advertiserId @@ -148,10 +147,6 @@ const destination: AudienceDestinationDefinition = { const currency = audienceSettings?.currency const cpm_cents = audienceSettings?.cpmCents - const statsName = 'createAmazonAudience' - statsTags?.push(`slug:${destination.slug}`) - statsClient?.incr(`${statsName}.intialise`, 1, statsTags) - if (!advertiser_id) { throw new IntegrationError('Missing advertiserId Value', 'MISSING_REQUIRED_FIELD', 400) } @@ -203,11 +198,6 @@ const destination: AudienceDestinationDefinition = { }) } - // @ts-ignore - TS doesn't know about the oauth property - // const authSettings = getAuthSettings(settings) - - // const authToken = await getAuthToken(request, createAudienceInput.settings, authSettings) - let payloadString = JSON.stringify(payload) // Regular expression to find a advertiserId numeric string and replace the quoted advertiserId string with an unquoted number // AdvertiserId is very big number string and can not be assigned or converted to number directly as it changes the value due to integer overflow. @@ -218,14 +208,12 @@ const destination: AudienceDestinationDefinition = { body: payloadString, headers: { 'Content-Type': 'application/vnd.amcaudiences.v1+json' - // authorization: `Bearer ${authToken}` } }) const res = await response.text() // Regular expression to find a audienceId number and replace the audienceId with quoted string const resp = extractNumberAndSubstituteWithStringValue(res, REGEX_AUDIENCEID, '"audienceId":"$1"') - statsClient?.incr(`${statsName}.sucsess`, 1, statsTags) return { externalId: resp.audienceId @@ -235,28 +223,18 @@ const destination: AudienceDestinationDefinition = { async getAudience(request, getAudienceInput) { // getAudienceInput.externalId represents audience ID that was created in createAudience const audience_id = getAudienceInput.externalId - const { settings, statsContext } = getAudienceInput + const { settings } = getAudienceInput const endpoint = settings.region - const { statsClient, tags: statsTags } = statsContext || {} if (!audience_id) { throw new IntegrationError('Missing audienceId value', 'MISSING_REQUIRED_FIELD', 400) } - // @ts-ignore - TS doesn't know about the oauth property - // const authSettings = getAuthSettings(settings) - // const authToken = await getAuthToken(request, settings, authSettings) - const statsName = 'getAmazonAudience' - statsTags?.push(`slug:${destination.slug}`) - statsClient?.incr(`${statsName}.intialise`, 1, statsTags) + const response = await request(`${endpoint}/amc/audiences/metadata/${audience_id}`, { method: 'GET' - // headers: { - // authorization: `Bearer ${authToken}` - // } }) const res = await response.text() // Regular expression to find a audienceId number and replace the audienceId with quoted string const resp = extractNumberAndSubstituteWithStringValue(res, REGEX_AUDIENCEID, '"audienceId":"$1"') - statsClient?.incr(`${statsName}.success`, 1, statsTags) return { externalId: resp.audienceId } From 1b7590a71746f87ebf73c5d4021b16efe220b8de Mon Sep 17 00:00:00 2001 From: Gaurav Kochar Date: Thu, 4 Jul 2024 13:45:16 +0530 Subject: [PATCH 04/10] Removed commented and unnecessary code --- packages/core/src/destination-kit/index.ts | 2 +- .../amazon-amc/__tests__/index.test.ts | 19 ------------------- .../src/destinations/amazon-amc/index.ts | 3 --- 3 files changed, 1 insertion(+), 23 deletions(-) diff --git a/packages/core/src/destination-kit/index.ts b/packages/core/src/destination-kit/index.ts index 29f03da02d..4c1abb4320 100644 --- a/packages/core/src/destination-kit/index.ts +++ b/packages/core/src/destination-kit/index.ts @@ -482,7 +482,7 @@ export class Destination { } const opts = this.extendRequest?.(context) ?? {} const requestClient = createRequestClient({ ...opts, statsContext: context.statsContext }) - return audienceDefinition.audienceConfig?.getAudience(requestClient, getAudienceInput) + return await audienceDefinition.audienceConfig?.getAudience(requestClient, getAudienceInput) } const onFailedAttempt = async (error: ResponseError & HTTPError) => { diff --git a/packages/destination-actions/src/destinations/amazon-amc/__tests__/index.test.ts b/packages/destination-actions/src/destinations/amazon-amc/__tests__/index.test.ts index 0ff6d01a42..d72322169f 100644 --- a/packages/destination-actions/src/destinations/amazon-amc/__tests__/index.test.ts +++ b/packages/destination-actions/src/destinations/amazon-amc/__tests__/index.test.ts @@ -116,15 +116,6 @@ describe('Amazon-Ads (actions)', () => { ) }) - // it('should fail if refresh token API gets failed', async () => { - // const endpoint = AUTHORIZATION_URL[`${settings.region}`] - // nock(`${endpoint}`).post('/auth/o2/token').reply(401) - - // await expect(testDestination.createAudience(createAudienceInputTemp)).rejects.toThrowError( - // InvalidAuthenticationError - // ) - // }) - it('should throw an HTTPError when createAudience API response is not ok', async () => { const endpoint = AUTHORIZATION_URL[`${settings.region}`] nock(`${endpoint}`).post('/auth/o2/token').reply(200) @@ -138,9 +129,6 @@ describe('Amazon-Ads (actions)', () => { }) it('creates an audience', async () => { - const endpoint = AUTHORIZATION_URL[`${settings.region}`] - nock(`${endpoint}`).post('/auth/o2/token').reply(200) - nock(`${settings.region}`) .post('/amc/audiences/metadata') .matchHeader('content-type', 'application/vnd.amcaudiences.v1+json') @@ -205,13 +193,6 @@ describe('Amazon-Ads (actions)', () => { await expect(audiencePromise).rejects.toHaveProperty('response.statusText', 'Not Found') await expect(audiencePromise).rejects.toHaveProperty('response.status', 404) }) - // it('should fail if refresh token API gets failed ', async () => { - // const endpoint = AUTHORIZATION_URL[`${settings.region}`] - // nock(`${endpoint}`).post('/auth/o2/token').reply(401) - - // const audiencePromise = testDestination.getAudience(getAudienceInput) - // await expect(audiencePromise).rejects.toThrow(InvalidAuthenticationError) - // }) it('should throw an IntegrationError when the audienceId is not provided', async () => { getAudienceInput.externalId = '' diff --git a/packages/destination-actions/src/destinations/amazon-amc/index.ts b/packages/destination-actions/src/destinations/amazon-amc/index.ts index c8287d2a97..43f66f24b5 100644 --- a/packages/destination-actions/src/destinations/amazon-amc/index.ts +++ b/packages/destination-actions/src/destinations/amazon-amc/index.ts @@ -4,7 +4,6 @@ import type { Settings, AudienceSettings } from './generated-types' import { AudiencePayload, extractNumberAndSubstituteWithStringValue, - // getAuthSettings, getAuthToken, REGEX_ADVERTISERID, REGEX_AUDIENCEID @@ -214,7 +213,6 @@ const destination: AudienceDestinationDefinition = { const res = await response.text() // Regular expression to find a audienceId number and replace the audienceId with quoted string const resp = extractNumberAndSubstituteWithStringValue(res, REGEX_AUDIENCEID, '"audienceId":"$1"') - return { externalId: resp.audienceId } @@ -228,7 +226,6 @@ const destination: AudienceDestinationDefinition = { if (!audience_id) { throw new IntegrationError('Missing audienceId value', 'MISSING_REQUIRED_FIELD', 400) } - const response = await request(`${endpoint}/amc/audiences/metadata/${audience_id}`, { method: 'GET' }) From 631d1dd86d8b2ecb7a3ce36a9c750d4ed12ed134 Mon Sep 17 00:00:00 2001 From: Gaurav Kochar Date: Tue, 9 Jul 2024 17:49:38 +0530 Subject: [PATCH 05/10] Fixed AudienceDestinationDefinition --- packages/core/src/destination-kit/index.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/core/src/destination-kit/index.ts b/packages/core/src/destination-kit/index.ts index 4c1abb4320..e26a1e10ca 100644 --- a/packages/core/src/destination-kit/index.ts +++ b/packages/core/src/destination-kit/index.ts @@ -140,8 +140,9 @@ const instanceOfAudienceDestinationSettingsWithCreateGet = ( export interface AudienceDestinationDefinition extends DestinationDefinition { - audienceConfig: AudienceDestinationConfigurationWithCreateGet - // | AudienceDestinationConfiguration + audienceConfig: + | AudienceDestinationConfigurationWithCreateGet + | AudienceDestinationConfiguration audienceFields: Record @@ -422,8 +423,8 @@ export class Destination { async createAudience(createAudienceInput: CreateAudienceInput) { let settings: JSONObject = createAudienceInput.settings as unknown as JSONObject - const audienceDefinition = this.definition as AudienceDestinationDefinition - if (!instanceOfAudienceDestinationSettingsWithCreateGet(audienceDefinition.audienceConfig)) { + const { audienceConfig } = this.definition as AudienceDestinationDefinition + if (!instanceOfAudienceDestinationSettingsWithCreateGet(audienceConfig)) { throw new Error('Unexpected call to createAudience') } const destinationSettings = this.getDestinationSettings(settings) @@ -437,7 +438,7 @@ export class Destination { } const opts = this.extendRequest?.(context) ?? {} const requestClient = createRequestClient({ ...opts, statsContext: context.statsContext }) - return await audienceDefinition.audienceConfig?.createAudience(requestClient, createAudienceInput) + return await audienceConfig?.createAudience(requestClient, createAudienceInput) } const onFailedAttempt = async (error: ResponseError & HTTPError) => { @@ -466,9 +467,9 @@ export class Destination { } async getAudience(getAudienceInput: GetAudienceInput) { - const audienceDefinition = this.definition as AudienceDestinationDefinition + const { audienceConfig } = this.definition as AudienceDestinationDefinition let settings: JSONObject = getAudienceInput.settings as unknown as JSONObject - if (!instanceOfAudienceDestinationSettingsWithCreateGet(audienceDefinition.audienceConfig)) { + if (!instanceOfAudienceDestinationSettingsWithCreateGet(audienceConfig)) { throw new Error('Unexpected call to getAudience') } const destinationSettings = this.getDestinationSettings(settings) @@ -482,7 +483,7 @@ export class Destination { } const opts = this.extendRequest?.(context) ?? {} const requestClient = createRequestClient({ ...opts, statsContext: context.statsContext }) - return await audienceDefinition.audienceConfig?.getAudience(requestClient, getAudienceInput) + return await audienceConfig?.getAudience(requestClient, getAudienceInput) } const onFailedAttempt = async (error: ResponseError & HTTPError) => { From 4730bec431e70e8f49ef96223a16bf130d695fa6 Mon Sep 17 00:00:00 2001 From: Gaurav Kochar Date: Wed, 24 Jul 2024 12:34:17 +0530 Subject: [PATCH 06/10] Made runFailedAttempt as reusable function across all execute blocks --- packages/core/src/destination-kit/index.ts | 121 ++++++--------------- 1 file changed, 31 insertions(+), 90 deletions(-) diff --git a/packages/core/src/destination-kit/index.ts b/packages/core/src/destination-kit/index.ts index e26a1e10ca..3be7e94166 100644 --- a/packages/core/src/destination-kit/index.ts +++ b/packages/core/src/destination-kit/index.ts @@ -442,26 +442,7 @@ export class Destination { } const onFailedAttempt = async (error: ResponseError & HTTPError) => { - const statusCode = error?.status ?? error?.response?.status ?? 500 - - // Throw original error if it is unrelated to invalid access tokens and not an oauth2 scheme - if ( - !( - statusCode === 401 && - (this.authentication?.scheme === 'oauth2' || this.authentication?.scheme === 'oauth-managed') - ) - ) { - throw error - } - - const oauthSettings = getOAuth2Data(settings) - const newTokens = await this.refreshAccessToken(destinationSettings, oauthSettings) - if (!newTokens) { - throw new InvalidAuthenticationError('Failed to refresh access token', ErrorCodes.OAUTH_REFRESH_FAILED) - } - - // Update `settings` with new tokens - settings = updateOAuthSettings(settings, newTokens) + settings = await this.runFailedAttempt(error, settings) } return await retry(run, { retries: 2, onFailedAttempt }) } @@ -487,26 +468,7 @@ export class Destination { } const onFailedAttempt = async (error: ResponseError & HTTPError) => { - const statusCode = error?.status ?? error?.response?.status ?? 500 - - // Throw original error if it is unrelated to invalid access tokens and not an oauth2 scheme - if ( - !( - statusCode === 401 && - (this.authentication?.scheme === 'oauth2' || this.authentication?.scheme === 'oauth-managed') - ) - ) { - throw error - } - - const oauthSettings = getOAuth2Data(settings) - const newTokens = await this.refreshAccessToken(destinationSettings, oauthSettings) - if (!newTokens) { - throw new InvalidAuthenticationError('Failed to refresh access token', ErrorCodes.OAUTH_REFRESH_FAILED) - } - - // Update `settings` with new tokens - settings = updateOAuthSettings(settings, newTokens) + settings = await this.runFailedAttempt(error, settings) } return await retry(run, { retries: 2, onFailedAttempt }) @@ -806,31 +768,7 @@ export class Destination { } const onFailedAttempt = async (error: ResponseError & HTTPError) => { - const statusCode = error?.status ?? error?.response?.status ?? 500 - - // Throw original error if it is unrelated to invalid access tokens and not an oauth2 scheme - if ( - !( - statusCode === 401 && - (this.authentication?.scheme === 'oauth2' || this.authentication?.scheme === 'oauth-managed') - ) - ) { - throw error - } - - const oauthSettings = getOAuth2Data(settings) - const newTokens = await this.refreshAccessToken( - destinationSettings, - oauthSettings, - options?.synchronizeRefreshAccessToken - ) - if (!newTokens) { - throw new InvalidAuthenticationError('Failed to refresh access token', ErrorCodes.OAUTH_REFRESH_FAILED) - } - - // Update `settings` with new tokens - settings = updateOAuthSettings(settings, newTokens) - await options?.onTokenRefresh?.(newTokens) + settings = await this.runFailedAttempt(error, settings, options) } return await retry(run, { retries: 2, onFailedAttempt }) @@ -858,31 +796,7 @@ export class Destination { // eslint-disable-next-line @typescript-eslint/no-explicit-any const onFailedAttempt = async (error: any) => { - const statusCode = error?.status ?? error?.response?.status ?? 500 - - // Throw original error if it is unrelated to invalid access tokens and not an oauth2 scheme - if ( - !( - statusCode === 401 && - (this.authentication?.scheme === 'oauth2' || this.authentication?.scheme === 'oauth-managed') - ) - ) { - throw error - } - - const oauthSettings = getOAuth2Data(settings) - const newTokens = await this.refreshAccessToken( - destinationSettings, - oauthSettings, - options?.synchronizeRefreshAccessToken - ) - if (!newTokens) { - throw new InvalidAuthenticationError('Failed to refresh access token', ErrorCodes.OAUTH_REFRESH_FAILED) - } - - // Update `settings` with new tokens - settings = updateOAuthSettings(settings, newTokens) - await options?.onTokenRefresh?.(newTokens) + settings = await this.runFailedAttempt(error, settings) } return await retry(run, { retries: 2, onFailedAttempt }) @@ -910,4 +824,31 @@ export class Destination { const { subcription, subscriptions, oauth, ...otherSettings } = settings return otherSettings as unknown as Settings } + // Refreshes the token and update it in setting in case of 401(Unauthorized). + async runFailedAttempt(error: ResponseError & HTTPError, settings: JSONObject, options?: OnEventOptions) { + const statusCode = error?.status ?? error?.response?.status ?? 500 + // Throw original error if it is unrelated to invalid access tokens and not an oauth2 scheme + if ( + !( + statusCode === 401 && + (this.authentication?.scheme === 'oauth2' || this.authentication?.scheme === 'oauth-managed') + ) + ) { + throw error + } + const destinationSettings = this.getDestinationSettings(settings) + const oauthSettings = getOAuth2Data(settings) + const newTokens = await this.refreshAccessToken( + destinationSettings, + oauthSettings, + options?.synchronizeRefreshAccessToken + ) + if (!newTokens) { + throw new InvalidAuthenticationError('Failed to refresh access token', ErrorCodes.OAUTH_REFRESH_FAILED) + } + + // Update `settings` with new tokens + await options?.onTokenRefresh?.(newTokens) + return updateOAuthSettings(settings, newTokens) + } } From f04d88bb00bf633328d89d19831c2ff4aa6e449e Mon Sep 17 00:00:00 2001 From: Gaurav Kochar Date: Thu, 25 Jul 2024 17:07:41 +0530 Subject: [PATCH 07/10] Added a unit test case for reauthentication flow for createAudience and getAudience --- .../src/__tests__/destination-kit.test.ts | 156 +++++++++++++++++- 1 file changed, 155 insertions(+), 1 deletion(-) diff --git a/packages/core/src/__tests__/destination-kit.test.ts b/packages/core/src/__tests__/destination-kit.test.ts index 6135405a4a..ba40bb4426 100644 --- a/packages/core/src/__tests__/destination-kit.test.ts +++ b/packages/core/src/__tests__/destination-kit.test.ts @@ -5,7 +5,9 @@ import { Logger, StatsClient, StatsContext, - TransactionContext + TransactionContext, + AudienceDestinationDefinition, + AuthenticationScheme } from '../destination-kit' import { JSONObject } from '../json-object' import { SegmentEvent } from '../segment-event' @@ -77,6 +79,68 @@ const destinationOAuth2: DestinationDefinition = { } } +const authentication: AuthenticationScheme = { + scheme: 'oauth2', + fields: {}, + refreshAccessToken: (_request) => { + return new Promise((resolve, _reject) => { + setTimeout(() => { + resolve({ + accessToken: 'fresh-token' + }) + }, 3) + }) + } +} + +const audienceDestination: AudienceDestinationDefinition = { + name: 'Amazon AMC (Actions)', + mode: 'cloud', + authentication: authentication, + audienceFields: {}, + audienceConfig: { + mode: { + type: 'synced', // Indicates that the audience is synced on some schedule; update as necessary + full_audience_sync: false // If true, we send the entire audience. If false, we just send the delta. + }, + async createAudience(_request, createAudienceInput) { + const settings: any = createAudienceInput.settings + + if (settings.oauth.access_token == 'fresh-token' || settings.oauth.access_token == 'valid-access-token') { + return new Promise((resolve, _reject) => { + setTimeout(() => { + resolve({ externalId: '123456789' }) + }, 3) + }) + } + + return new Promise((_resolve, reject) => { + setTimeout(() => { + reject({ status: 401 }) + }, 3) + }) + }, + async getAudience(_request, getAudienceInput) { + const settings: any = getAudienceInput.settings + + if (settings.oauth.access_token == 'fresh-token' || settings.oauth.access_token == 'valid-access-token') { + return new Promise((resolve, _reject) => { + setTimeout(() => { + resolve({ externalId: getAudienceInput.externalId }) + }, 3) + }) + } + + return new Promise((_resolve, reject) => { + setTimeout(() => { + reject({ status: 401 }) + }, 3) + }) + } + }, + actions: {} +} + const destinationWithOptions: DestinationDefinition = { name: 'Actions Google Analytic 4', mode: 'cloud', @@ -953,4 +1017,94 @@ describe('destination kit', () => { }) }) }) + + describe('Audience Destination', () => { + beforeEach(async () => { + jest.restoreAllMocks() + jest.resetAllMocks() + }) + describe('createAudience', () => { + test('Refreshes the access-token in case of Unauthorized(401)', async () => { + const createAudienceInput = { + audienceName: 'Test Audience', + settings: { + oauth: { + clientId: 'valid-client-id', + clientSecret: 'valid-client-secret', + access_token: 'expired-access-token', + refresh_token: 'refresh-token', + token_type: 'bearer' + } + } + } + const destinationTest = new Destination(audienceDestination) + const spy = jest.spyOn(authentication, 'refreshAccessToken') + const res = await destinationTest.createAudience(createAudienceInput) + expect(res).toEqual({ externalId: '123456789' }) + expect(spy).toHaveBeenCalledTimes(1) + }) + + test('Will not refresh access-token if token is already valid', async () => { + const createAudienceInput = { + audienceName: 'Test Audience', + settings: { + oauth: { + clientId: 'valid-client-id', + clientSecret: 'valid-client-secret', + access_token: 'valid-access-token', + refresh_token: 'refresh-token', + token_type: 'bearer' + } + } + } + const destinationTest = new Destination(audienceDestination) + const spy = jest.spyOn(authentication, 'refreshAccessToken') + const res = await destinationTest.createAudience(createAudienceInput) + expect(res).toEqual({ externalId: '123456789' }) + expect(spy).not.toHaveBeenCalled() + }) + }) + + describe('getAudience', () => { + test('Refreshes the access-token in case of Unauthorized(401)', async () => { + const createAudienceInput = { + externalId: '366170701270726115', + settings: { + oauth: { + clientId: 'valid-client-id', + clientSecret: 'valid-client-secret', + access_token: 'expired-access-token', + refresh_token: 'refresh-token', + token_type: 'bearer' + } + } + } + const destinationTest = new Destination(audienceDestination) + const spy = jest.spyOn(authentication, 'refreshAccessToken') + const res = await destinationTest.getAudience(createAudienceInput) + expect(res).toEqual({ externalId: '366170701270726115' }) + expect(spy).toHaveBeenCalledTimes(1) + }) + + test('Will not refresh access-token if token is already valid', async () => { + const createAudienceInput = { + externalId: '366170701270726115', + settings: { + oauth: { + clientId: 'valid-client-id', + clientSecret: 'valid-client-secret', + access_token: 'valid-access-token', + refresh_token: 'refresh-token', + token_type: 'bearer' + } + } + } + const destinationTest = new Destination(audienceDestination) + const spy = jest.spyOn(authentication, 'refreshAccessToken') + const res = await destinationTest.getAudience(createAudienceInput) + expect(res).toEqual({ externalId: '366170701270726115' }) + expect(spy).not.toHaveBeenCalled() + }) + }) + }) }) From 114e732717788511d71a3e98be2d524c25db0836 Mon Sep 17 00:00:00 2001 From: Gaurav Kochar Date: Thu, 25 Jul 2024 17:36:50 +0530 Subject: [PATCH 08/10] added comment --- packages/core/src/__tests__/destination-kit.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core/src/__tests__/destination-kit.test.ts b/packages/core/src/__tests__/destination-kit.test.ts index ba40bb4426..c4c3003b64 100644 --- a/packages/core/src/__tests__/destination-kit.test.ts +++ b/packages/core/src/__tests__/destination-kit.test.ts @@ -103,6 +103,7 @@ const audienceDestination: AudienceDestinationDefinition = { type: 'synced', // Indicates that the audience is synced on some schedule; update as necessary full_audience_sync: false // If true, we send the entire audience. If false, we just send the delta. }, + // Mocked createAudience Handler async createAudience(_request, createAudienceInput) { const settings: any = createAudienceInput.settings @@ -120,6 +121,7 @@ const audienceDestination: AudienceDestinationDefinition = { }, 3) }) }, + // Mocked getAudience Handler async getAudience(_request, getAudienceInput) { const settings: any = getAudienceInput.settings From 8dfdabfd95ddceed4a333d926315b0a8c8e08e59 Mon Sep 17 00:00:00 2001 From: Gaurav Kochar Date: Wed, 31 Jul 2024 17:03:38 +0530 Subject: [PATCH 09/10] PR review comments:- Added unit test case for non-oauth scheme and non 401 --- .../src/__tests__/destination-kit.test.ts | 156 ++++++++++++++---- packages/core/src/destination-kit/index.ts | 14 +- 2 files changed, 136 insertions(+), 34 deletions(-) diff --git a/packages/core/src/__tests__/destination-kit.test.ts b/packages/core/src/__tests__/destination-kit.test.ts index c4c3003b64..7cb61432b1 100644 --- a/packages/core/src/__tests__/destination-kit.test.ts +++ b/packages/core/src/__tests__/destination-kit.test.ts @@ -1,3 +1,4 @@ +import { IntegrationError } from '../errors' import { StateContext, Destination, @@ -11,6 +12,7 @@ import { } from '../destination-kit' import { JSONObject } from '../json-object' import { SegmentEvent } from '../segment-event' +const WRONG_AUDIENCE_ID = '1234567890' const destinationCustomAuth: DestinationDefinition = { name: 'Actions Google Analytic 4', @@ -84,11 +86,9 @@ const authentication: AuthenticationScheme = { fields: {}, refreshAccessToken: (_request) => { return new Promise((resolve, _reject) => { - setTimeout(() => { - resolve({ - accessToken: 'fresh-token' - }) - }, 3) + resolve({ + accessToken: 'fresh-token' + }) }) } } @@ -103,40 +103,45 @@ const audienceDestination: AudienceDestinationDefinition = { type: 'synced', // Indicates that the audience is synced on some schedule; update as necessary full_audience_sync: false // If true, we send the entire audience. If false, we just send the delta. }, + // Mocked createAudience Handler async createAudience(_request, createAudienceInput) { const settings: any = createAudienceInput.settings + const audienceSettings: any = createAudienceInput.audienceSettings - if (settings.oauth.access_token == 'fresh-token' || settings.oauth.access_token == 'valid-access-token') { - return new Promise((resolve, _reject) => { - setTimeout(() => { - resolve({ externalId: '123456789' }) - }, 3) + // it could be due to invalid input or Bad Request + if (!audienceSettings?.advertiserId) + throw new IntegrationError('Missing advertiserId Value', 'MISSING_REQUIRED_FIELD', 400) + + // invalid access token + if (settings.oauth.access_token == 'invalid-access-token' || settings.oauth.clientId == 'invalid_client_id') { + return new Promise((_resolve, reject) => { + reject(new IntegrationError('Unauthorized', 'UNAUTHORIZED', 401)) }) } - return new Promise((_resolve, reject) => { - setTimeout(() => { - reject({ status: 401 }) - }, 3) + return new Promise((resolve, _reject) => { + resolve({ externalId: '123456789' }) }) }, + // Mocked getAudience Handler async getAudience(_request, getAudienceInput) { const settings: any = getAudienceInput.settings + const audience_id = getAudienceInput.externalId + + if (audience_id == WRONG_AUDIENCE_ID) { + throw new IntegrationError('audienceId not found', 'AUDIENCEID_NOT_FOUND', 400) + } - if (settings.oauth.access_token == 'fresh-token' || settings.oauth.access_token == 'valid-access-token') { - return new Promise((resolve, _reject) => { - setTimeout(() => { - resolve({ externalId: getAudienceInput.externalId }) - }, 3) + if (settings.oauth.access_token == 'invalid-access-token' || settings.oauth.clientId == 'invalid_client_id') { + return new Promise((_resolve, reject) => { + reject(new IntegrationError('Unauthorized', 'UNAUTHORIZED', 401)) }) } - return new Promise((_resolve, reject) => { - setTimeout(() => { - reject({ status: 401 }) - }, 3) + return new Promise((resolve, _reject) => { + resolve({ externalId: audience_id }) }) } }, @@ -1033,10 +1038,13 @@ describe('destination kit', () => { oauth: { clientId: 'valid-client-id', clientSecret: 'valid-client-secret', - access_token: 'expired-access-token', + access_token: 'invalid-access-token', refresh_token: 'refresh-token', token_type: 'bearer' } + }, + audienceSettings: { + advertiserId: '12334745462532' } } const destinationTest = new Destination(audienceDestination) @@ -1046,6 +1054,26 @@ describe('destination kit', () => { expect(spy).toHaveBeenCalledTimes(1) }) + test('Will not refresh access-token in case of any non 401 error', async () => { + const createAudienceInput = { + audienceName: 'Test Audience', + settings: { + oauth: { + clientId: 'valid-client-id', + clientSecret: 'valid-client-secret', + access_token: 'access-token', + refresh_token: 'refresh-token', + token_type: 'bearer' + } + }, + audienceSettings: {} + } + const destinationTest = new Destination(audienceDestination) + const spy = jest.spyOn(authentication, 'refreshAccessToken') + await expect(destinationTest.createAudience(createAudienceInput)).rejects.toThrowError() + expect(spy).not.toHaveBeenCalled() + }) + test('Will not refresh access-token if token is already valid', async () => { const createAudienceInput = { audienceName: 'Test Audience', @@ -1057,39 +1085,87 @@ describe('destination kit', () => { refresh_token: 'refresh-token', token_type: 'bearer' } + }, + audienceSettings: { + advertiserId: '12334745462532' } } + const destinationTest = new Destination(audienceDestination) const spy = jest.spyOn(authentication, 'refreshAccessToken') const res = await destinationTest.createAudience(createAudienceInput) expect(res).toEqual({ externalId: '123456789' }) expect(spy).not.toHaveBeenCalled() }) + + test('Will not refresh the access-token for non-Oauth authentication scheme', async () => { + const createAudienceInput = { + audienceName: 'Test Audience', + settings: { + oauth: { + clientId: 'invalid_client_id', + clientSecret: 'valid-client-secret' + } + }, + audienceSettings: { + advertiserId: '12334745462532' + } + } + // Non-Oauth authentication scheme + audienceDestination.authentication = { + scheme: 'custom', + fields: {} + } + const destinationTest = new Destination(audienceDestination) + const spy = jest.spyOn(authentication, 'refreshAccessToken') + await expect(destinationTest.createAudience(createAudienceInput)).rejects.toThrowError() + expect(spy).not.toHaveBeenCalled() + }) }) describe('getAudience', () => { test('Refreshes the access-token in case of Unauthorized(401)', async () => { - const createAudienceInput = { + const getAudienceInput = { externalId: '366170701270726115', settings: { oauth: { clientId: 'valid-client-id', clientSecret: 'valid-client-secret', - access_token: 'expired-access-token', + access_token: 'invalid-access-token', refresh_token: 'refresh-token', token_type: 'bearer' } } } + audienceDestination.authentication = authentication const destinationTest = new Destination(audienceDestination) const spy = jest.spyOn(authentication, 'refreshAccessToken') - const res = await destinationTest.getAudience(createAudienceInput) + const res = await destinationTest.getAudience(getAudienceInput) expect(res).toEqual({ externalId: '366170701270726115' }) expect(spy).toHaveBeenCalledTimes(1) }) + test('Will not refresh access-token in case of any non 401 error', async () => { + const getAudienceInput = { + externalId: WRONG_AUDIENCE_ID, + settings: { + oauth: { + clientId: 'valid-client-id', + clientSecret: 'valid-client-secret', + access_token: 'valid-access-token', + refresh_token: 'refresh-token', + token_type: 'bearer' + } + } + } + const destinationTest = new Destination(audienceDestination) + const spy = jest.spyOn(authentication, 'refreshAccessToken') + await expect(destinationTest.getAudience(getAudienceInput)).rejects.toThrowError() + expect(spy).not.toHaveBeenCalled() + }) + test('Will not refresh access-token if token is already valid', async () => { - const createAudienceInput = { + const getAudienceInput = { externalId: '366170701270726115', settings: { oauth: { @@ -1103,10 +1179,32 @@ describe('destination kit', () => { } const destinationTest = new Destination(audienceDestination) const spy = jest.spyOn(authentication, 'refreshAccessToken') - const res = await destinationTest.getAudience(createAudienceInput) + const res = await destinationTest.getAudience(getAudienceInput) expect(res).toEqual({ externalId: '366170701270726115' }) expect(spy).not.toHaveBeenCalled() }) + + test('Will not refresh the access-token for non-Oauth authentication scheme', async () => { + const getAudienceInput = { + externalId: '366170701270726115', + settings: { + oauth: { + clientId: 'invalid_client_id', + clientSecret: 'valid-client-secret' + } + } + } + + // Non-Oauth authentication scheme + audienceDestination.authentication = { + scheme: 'custom', + fields: {} + } + const destinationTest = new Destination(audienceDestination) + const spy = jest.spyOn(authentication, 'refreshAccessToken') + await expect(destinationTest.getAudience(getAudienceInput)).rejects.toThrowError() + expect(spy).not.toHaveBeenCalled() + }) }) }) }) diff --git a/packages/core/src/destination-kit/index.ts b/packages/core/src/destination-kit/index.ts index 3be7e94166..a412504404 100644 --- a/packages/core/src/destination-kit/index.ts +++ b/packages/core/src/destination-kit/index.ts @@ -442,7 +442,7 @@ export class Destination { } const onFailedAttempt = async (error: ResponseError & HTTPError) => { - settings = await this.runFailedAttempt(error, settings) + settings = await this.refreshAndUpdateTokenInSettings(error, settings) } return await retry(run, { retries: 2, onFailedAttempt }) } @@ -468,7 +468,7 @@ export class Destination { } const onFailedAttempt = async (error: ResponseError & HTTPError) => { - settings = await this.runFailedAttempt(error, settings) + settings = await this.refreshAndUpdateTokenInSettings(error, settings) } return await retry(run, { retries: 2, onFailedAttempt }) @@ -768,7 +768,7 @@ export class Destination { } const onFailedAttempt = async (error: ResponseError & HTTPError) => { - settings = await this.runFailedAttempt(error, settings, options) + settings = await this.refreshAndUpdateTokenInSettings(error, settings, options) } return await retry(run, { retries: 2, onFailedAttempt }) @@ -796,7 +796,7 @@ export class Destination { // eslint-disable-next-line @typescript-eslint/no-explicit-any const onFailedAttempt = async (error: any) => { - settings = await this.runFailedAttempt(error, settings) + settings = await this.refreshAndUpdateTokenInSettings(error, settings) } return await retry(run, { retries: 2, onFailedAttempt }) @@ -825,7 +825,11 @@ export class Destination { return otherSettings as unknown as Settings } // Refreshes the token and update it in setting in case of 401(Unauthorized). - async runFailedAttempt(error: ResponseError & HTTPError, settings: JSONObject, options?: OnEventOptions) { + async refreshAndUpdateTokenInSettings( + error: ResponseError & HTTPError, + settings: JSONObject, + options?: OnEventOptions + ) { const statusCode = error?.status ?? error?.response?.status ?? 500 // Throw original error if it is unrelated to invalid access tokens and not an oauth2 scheme if ( From 34352be3b29c50b63fdf5694611c44a0b74f654e Mon Sep 17 00:00:00 2001 From: Gaurav Kochar Date: Mon, 5 Aug 2024 15:54:35 +0530 Subject: [PATCH 10/10] Split failedAttemptHandler into three seperate methods --- packages/core/src/destination-kit/index.ts | 76 ++++++++++++++++------ 1 file changed, 57 insertions(+), 19 deletions(-) diff --git a/packages/core/src/destination-kit/index.ts b/packages/core/src/destination-kit/index.ts index f14d04ff79..8b9d278e06 100644 --- a/packages/core/src/destination-kit/index.ts +++ b/packages/core/src/destination-kit/index.ts @@ -442,7 +442,7 @@ export class Destination { } const onFailedAttempt = async (error: ResponseError & HTTPError) => { - settings = await this.refreshAndUpdateTokenInSettings(error, settings) + settings = await this.handleAuthError(error, settings) } return await retry(run, { retries: 2, onFailedAttempt }) } @@ -468,7 +468,7 @@ export class Destination { } const onFailedAttempt = async (error: ResponseError & HTTPError) => { - settings = await this.refreshAndUpdateTokenInSettings(error, settings) + settings = await this.handleAuthError(error, settings) } return await retry(run, { retries: 2, onFailedAttempt }) @@ -770,7 +770,7 @@ export class Destination { } const onFailedAttempt = async (error: ResponseError & HTTPError) => { - settings = await this.refreshAndUpdateTokenInSettings(error, settings, options) + settings = await this.handleAuthError(error, settings, options) } return await retry(run, { retries: 2, onFailedAttempt }) @@ -798,7 +798,7 @@ export class Destination { // eslint-disable-next-line @typescript-eslint/no-explicit-any const onFailedAttempt = async (error: any) => { - settings = await this.refreshAndUpdateTokenInSettings(error, settings) + settings = await this.handleAuthError(error, settings) } return await retry(run, { retries: 2, onFailedAttempt }) @@ -826,22 +826,45 @@ export class Destination { const { subcription, subscriptions, oauth, ...otherSettings } = settings return otherSettings as unknown as Settings } - // Refreshes the token and update it in setting in case of 401(Unauthorized). - async refreshAndUpdateTokenInSettings( - error: ResponseError & HTTPError, - settings: JSONObject, - options?: OnEventOptions - ) { - const statusCode = error?.status ?? error?.response?.status ?? 500 - // Throw original error if it is unrelated to invalid access tokens and not an oauth2 scheme - if ( - !( - statusCode === 401 && - (this.authentication?.scheme === 'oauth2' || this.authentication?.scheme === 'oauth-managed') - ) - ) { + + /** + * Handles the failed attempt by checking if reauthentication is needed and updating the token if necessary. + * @param {ResponseError & HTTPError} error - The error object from the failed attempt. + * @param {JSONObject} settings - The current settings object. + * @returns {Promise} - The updated settings object. + * @throws {ResponseError & HTTPError} - If reauthentication is not needed or token refresh fails. + */ + async handleAuthError(error: ResponseError & HTTPError, settings: JSONObject, options?: OnEventOptions) { + if (this.needsReauthentication(error)) { + const newTokens = await this.refreshTokenAndGetNewToken(settings) + settings = await this.updateTokensInSettings(settings, newTokens, options) + } else { throw error } + return settings + } + + /** + * Determines if reauthentication is needed based on the error status. + * @param {ResponseError & HTTPError} error - The error object containing response details. + * @returns {boolean} - True if reauthentication is needed, otherwise false. + */ + needsReauthentication(error: ResponseError & HTTPError): boolean { + const statusCode = error?.status ?? error?.response?.status ?? 500 + return ( + statusCode === 401 && + (this.authentication?.scheme === 'oauth2' || this.authentication?.scheme === 'oauth-managed') + ) + } + + /** + * Refreshes the token and retrieves new tokens. + * @param {JSONObject} settings - The current settings object. + * @param {OnEventOptions} [options] - Optional event options for synchronizing token refresh. + * @returns {Promise} - The new tokens object. + * @throws {InvalidAuthenticationError} - If token refresh fails. + */ + async refreshTokenAndGetNewToken(settings: JSONObject, options?: OnEventOptions): Promise { const destinationSettings = this.getDestinationSettings(settings) const oauthSettings = getOAuth2Data(settings) const newTokens = await this.refreshAccessToken( @@ -849,11 +872,26 @@ export class Destination { oauthSettings, options?.synchronizeRefreshAccessToken ) + if (!newTokens) { throw new InvalidAuthenticationError('Failed to refresh access token', ErrorCodes.OAUTH_REFRESH_FAILED) } - // Update `settings` with new tokens + return newTokens + } + + /** + * Updates the settings object with new tokens. + * @param {JSONObject} settings - The current settings object. + * @param {RefreshAccessTokenResult} newTokens - The new tokens object. + * @param {OnEventOptions} [options] - Optional event options for handling token refresh. + * @returns {Promise} - The updated settings object. + */ + async updateTokensInSettings( + settings: JSONObject, + newTokens: RefreshAccessTokenResult, + options?: OnEventOptions + ): Promise { await options?.onTokenRefresh?.(newTokens) return updateOAuthSettings(settings, newTokens) }