From 7e928fe78a8d29bf16143366ec7b4d72e5727f5d Mon Sep 17 00:00:00 2001 From: Varadarajan V <109586712+varadarajan-tw@users.noreply.github.com> Date: Thu, 8 Aug 2024 23:01:17 +0530 Subject: [PATCH] =?UTF-8?q?Revert=20"[STRAT-3865]=20|=20Re-authentication?= =?UTF-8?q?=20Flow=20for=20CreateAudience=20and=20GetAudie=E2=80=A6"=20(#2?= =?UTF-8?q?262)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 3e8da1f25a6bd8bdf837941b04953b62527d4bea. --- .../src/__tests__/destination-kit.test.ts | 256 +----------------- packages/core/src/destination-kit/index.ts | 186 +++++-------- .../amazon-amc/__tests__/index.test.ts | 21 +- .../src/destinations/amazon-amc/index.ts | 17 +- 4 files changed, 111 insertions(+), 369 deletions(-) diff --git a/packages/core/src/__tests__/destination-kit.test.ts b/packages/core/src/__tests__/destination-kit.test.ts index a503355354..c59e01464c 100644 --- a/packages/core/src/__tests__/destination-kit.test.ts +++ b/packages/core/src/__tests__/destination-kit.test.ts @@ -1,4 +1,3 @@ -import { IntegrationError } from '../errors' import { ActionDefinition } from '../destination-kit/action' import { StateContext, @@ -7,13 +6,10 @@ import { Logger, StatsClient, StatsContext, - TransactionContext, - AudienceDestinationDefinition, - AuthenticationScheme + TransactionContext } 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', @@ -82,73 +78,6 @@ const destinationOAuth2: DestinationDefinition = { } } -const authentication: AuthenticationScheme = { - scheme: 'oauth2', - fields: {}, - refreshAccessToken: (_request) => { - return new Promise((resolve, _reject) => { - resolve({ - accessToken: 'fresh-token' - }) - }) - } -} - -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. - }, - - // Mocked createAudience Handler - async createAudience(_request, createAudienceInput) { - const settings: any = createAudienceInput.settings - const audienceSettings: any = createAudienceInput.audienceSettings - - // 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) => { - 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 == '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) => { - resolve({ externalId: audience_id }) - }) - } - }, - actions: {} -} - const destinationWithOptions: DestinationDefinition = { name: 'Actions Google Analytic 4', mode: 'cloud', @@ -1032,187 +961,4 @@ 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: 'invalid-access-token', - 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).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', - settings: { - oauth: { - clientId: 'valid-client-id', - clientSecret: 'valid-client-secret', - access_token: 'valid-access-token', - 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 getAudienceInput = { - externalId: '366170701270726115', - settings: { - oauth: { - clientId: 'valid-client-id', - clientSecret: 'valid-client-secret', - 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(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 getAudienceInput = { - 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(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 8b9d278e06..c7619f78c3 100644 --- a/packages/core/src/destination-kit/index.ts +++ b/packages/core/src/destination-kit/index.ts @@ -141,8 +141,8 @@ const instanceOfAudienceDestinationSettingsWithCreateGet = ( export interface AudienceDestinationDefinition extends DestinationDefinition { audienceConfig: - | AudienceDestinationConfigurationWithCreateGet | AudienceDestinationConfiguration + | AudienceDestinationConfigurationWithCreateGet audienceFields: Record @@ -422,56 +422,41 @@ export class Destination { } async createAudience(createAudienceInput: CreateAudienceInput) { - let settings: JSONObject = createAudienceInput.settings as unknown as JSONObject - const { audienceConfig } = this.definition as AudienceDestinationDefinition - if (!instanceOfAudienceDestinationSettingsWithCreateGet(audienceConfig)) { + const audienceDefinition = this.definition as AudienceDestinationDefinition + if (!instanceOfAudienceDestinationSettingsWithCreateGet(audienceDefinition.audienceConfig)) { throw new Error('Unexpected call to createAudience') } - const destinationSettings = this.getDestinationSettings(settings) - 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 audienceConfig?.createAudience(requestClient, createAudienceInput) + const destinationSettings = this.getDestinationSettings(createAudienceInput.settings as unknown as JSONObject) + const auth = getAuthData(createAudienceInput.settings as unknown as JSONObject) + const context: ExecuteInput = { + audienceSettings: createAudienceInput.audienceSettings, + settings: destinationSettings, + payload: undefined, + auth } + const options = this.extendRequest?.(context) ?? {} + const requestClient = createRequestClient({ ...options, statsContext: context.statsContext }) - const onFailedAttempt = async (error: ResponseError & HTTPError) => { - settings = await this.handleAuthError(error, settings) - } - return await retry(run, { retries: 2, onFailedAttempt }) + return audienceDefinition.audienceConfig?.createAudience(requestClient, createAudienceInput) } async getAudience(getAudienceInput: GetAudienceInput) { - const { audienceConfig } = this.definition as AudienceDestinationDefinition - let settings: JSONObject = getAudienceInput.settings as unknown as JSONObject - if (!instanceOfAudienceDestinationSettingsWithCreateGet(audienceConfig)) { + const audienceDefinition = this.definition as AudienceDestinationDefinition + if (!instanceOfAudienceDestinationSettingsWithCreateGet(audienceDefinition.audienceConfig)) { throw new Error('Unexpected call to getAudience') } - const destinationSettings = this.getDestinationSettings(settings) - 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 await audienceConfig?.getAudience(requestClient, getAudienceInput) - } - - const onFailedAttempt = async (error: ResponseError & HTTPError) => { - settings = await this.handleAuthError(error, settings) + const destinationSettings = this.getDestinationSettings(getAudienceInput.settings as unknown as JSONObject) + const auth = getAuthData(getAudienceInput.settings as unknown as JSONObject) + const context: ExecuteInput = { + audienceSettings: getAudienceInput.audienceSettings, + settings: destinationSettings, + payload: undefined, + auth } + const options = this.extendRequest?.(context) ?? {} + const requestClient = createRequestClient({ ...options, statsContext: context.statsContext }) - return await retry(run, { retries: 2, onFailedAttempt }) + return audienceDefinition.audienceConfig?.getAudience(requestClient, getAudienceInput) } async testAuthentication(settings: Settings): Promise { @@ -770,7 +755,31 @@ export class Destination { } const onFailedAttempt = async (error: ResponseError & HTTPError) => { - settings = await this.handleAuthError(error, settings, options) + 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) } return await retry(run, { retries: 2, onFailedAttempt }) @@ -798,7 +807,31 @@ export class Destination { // eslint-disable-next-line @typescript-eslint/no-explicit-any const onFailedAttempt = async (error: any) => { - settings = await this.handleAuthError(error, settings) + 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) } return await retry(run, { retries: 2, onFailedAttempt }) @@ -826,73 +859,4 @@ export class Destination { const { subcription, subscriptions, oauth, ...otherSettings } = settings return otherSettings as unknown as Settings } - - /** - * 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( - destinationSettings, - oauthSettings, - options?.synchronizeRefreshAccessToken - ) - - if (!newTokens) { - throw new InvalidAuthenticationError('Failed to refresh access token', ErrorCodes.OAUTH_REFRESH_FAILED) - } - - 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) - } } 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 d72322169f..2498008c66 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 } from '@segment/actions-core' +import { createTestIntegration, InvalidAuthenticationError } from '@segment/actions-core' import Definition from '../index' import { HTTPError } from '@segment/actions-core/*' import { AUTHORIZATION_URL } from '../utils' @@ -116,6 +116,15 @@ 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) @@ -129,6 +138,9 @@ 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') @@ -193,6 +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) + + 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 43f66f24b5..ff0244a582 100644 --- a/packages/destination-actions/src/destinations/amazon-amc/index.ts +++ b/packages/destination-actions/src/destinations/amazon-amc/index.ts @@ -4,6 +4,7 @@ import type { Settings, AudienceSettings } from './generated-types' import { AudiencePayload, extractNumberAndSubstituteWithStringValue, + getAuthSettings, getAuthToken, REGEX_ADVERTISERID, REGEX_AUDIENCEID @@ -197,6 +198,10 @@ 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. @@ -206,7 +211,8 @@ const destination: AudienceDestinationDefinition = { method: 'POST', body: payloadString, headers: { - 'Content-Type': 'application/vnd.amcaudiences.v1+json' + 'Content-Type': 'application/vnd.amcaudiences.v1+json', + authorization: `Bearer ${authToken}` } }) @@ -223,11 +229,18 @@ const destination: AudienceDestinationDefinition = { const audience_id = getAudienceInput.externalId const { settings } = getAudienceInput const endpoint = settings.region + 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 response = await request(`${endpoint}/amc/audiences/metadata/${audience_id}`, { - method: 'GET' + 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