From 7132d1d01be5ce8f31b44efdd316a60a3cb8a64b Mon Sep 17 00:00:00 2001 From: Brian Graves Date: Tue, 10 Feb 2026 15:50:57 -0800 Subject: [PATCH 1/4] Refactor Google Drive / Sheets oauth changes Refactor the oauth changes so we can extract some of the encryption logic for use with the remaining oauth actions. --- lib/actions/google/drive/google_drive.d.ts | 16 ++-- lib/actions/google/drive/google_drive.js | 44 ++++++---- lib/crypto/aes_transit_crypto.js | 6 +- lib/hub/action_token.d.ts | 6 -- lib/hub/action_token.js | 16 ---- lib/hub/encrypted_payload.d.ts | 8 ++ lib/hub/encrypted_payload.js | 32 +++++++ lib/hub/index.d.ts | 4 +- lib/hub/index.js | 4 +- lib/hub/oauth_action_v2.d.ts | 4 +- lib/hub/token_payload.d.ts | 4 + lib/hub/token_payload.js | 9 ++ src/actions/google/drive/drive_tokens.ts | 19 +++++ src/actions/google/drive/google_drive.ts | 97 +++++++++------------- src/crypto/aes_transit_crypto.ts | 6 +- src/hub/action_token.ts | 10 --- src/hub/encrypted_payload.ts | 43 ++++++++++ src/hub/index.ts | 4 +- src/hub/oauth_action_v2.ts | 6 +- src/hub/token_payload.ts | 7 ++ 20 files changed, 217 insertions(+), 128 deletions(-) delete mode 100644 lib/hub/action_token.d.ts delete mode 100644 lib/hub/action_token.js create mode 100644 lib/hub/token_payload.d.ts create mode 100644 lib/hub/token_payload.js create mode 100644 src/actions/google/drive/drive_tokens.ts delete mode 100644 src/hub/action_token.ts create mode 100644 src/hub/token_payload.ts diff --git a/lib/actions/google/drive/google_drive.d.ts b/lib/actions/google/drive/google_drive.d.ts index d0b1af351..bfd978843 100644 --- a/lib/actions/google/drive/google_drive.d.ts +++ b/lib/actions/google/drive/google_drive.d.ts @@ -7,6 +7,13 @@ interface OauthState { tokenurl?: string; stateurl?: string; } +declare class DriveTokens extends Hub.TokenPayload { + tokens: any; + redirect: string; + constructor(tokens: any, redirect: string); + static fromJson(json: any): DriveTokens; + asJson(): any; +} export declare class GoogleDriveAction extends Hub.OAuthActionV2 { name: string; label: string; @@ -30,7 +37,7 @@ export declare class GoogleDriveAction extends Hub.OAuthActionV2 { oauthHandleRedirect(urlParams: { [key: string]: string; }, redirectUri: string): Promise; - oauthFetchAccessToken(request: Hub.ActionRequest): Promise; + oauthFetchAccessToken(request: Hub.ActionRequest): Promise; oauthCheck(request: Hub.ActionRequest): Promise; oauth2Client(redirectUri: string | undefined): OAuth2Client; sendData(filename: string, request: Hub.ActionRequest, drive: Drive): Promise>; @@ -41,11 +48,10 @@ export declare class GoogleDriveAction extends Hub.OAuthActionV2 { protected driveClientFromRequest(redirect: string, tokens: Credentials): Promise; protected getUserEmail(redirect: string, tokens: Credentials): Promise; protected validateUserInDomainAllowlist(domainAllowlist: string | undefined, redirect: string, tokens: Credentials, requestWebhookId: string | undefined): Promise; - protected oauthExtractTokensFromStateJson(stateJson: string, requestWebhookId: string | undefined): Promise; + protected oauthExtractTokensFromStateJson(stateJson: string, requestWebhookId: string | undefined): Promise; protected validTokens(tokens: Credentials, requestWebhookId: string | undefined): boolean; - protected oauthMaybeEncryptTokens(tokenPayload: Hub.ActionToken, actionCrypto: Hub.ActionCrypto, requestWebhookId: string | undefined): Promise; - protected oauthEncryptTokens(tokenPayload: Hub.ActionToken, actionCrypto: Hub.ActionCrypto, requestWebhookId: string | undefined): Promise; - protected oauthDecryptTokens(encryptedPayload: Hub.EncryptedPayload, actionCrypto: Hub.ActionCrypto, requestWebhookId: string | undefined): Promise; + protected oauthMaybeEncryptTokens(tokenPayload: DriveTokens, requestWebhookId: string | undefined): Promise; + protected oauthDecryptTokens(encryptedPayload: Hub.EncryptedPayload, actionCrypto: Hub.ActionCrypto, requestWebhookId: string | undefined): Promise; protected oauthFetchAndStoreInfo(urlParams: { [key: string]: string; }, redirectUri: string, statePayload: OauthState, requestWebhookId: string | undefined): Promise; diff --git a/lib/actions/google/drive/google_drive.js b/lib/actions/google/drive/google_drive.js index 82ffb7e4f..f02cdfabb 100644 --- a/lib/actions/google/drive/google_drive.js +++ b/lib/actions/google/drive/google_drive.js @@ -11,6 +11,22 @@ const action_response_1 = require("../../../hub/action_response"); const domain_validator_1 = require("./domain_validator"); const LOG_PREFIX = "[GOOGLE_DRIVE]"; const FOLDERID_REGEX = /\/folders\/(?[^\/?]+)/; +class DriveTokens extends Hub.TokenPayload { + constructor(tokens, redirect) { + super(); + this.tokens = tokens; + this.redirect = redirect; + } + static fromJson(json) { + return new DriveTokens(json.tokens, json.redirect); + } + asJson() { + return { + tokens: this.tokens, + redirect: this.redirect, + }; + } +} class GoogleDriveAction extends Hub.OAuthActionV2 { constructor() { super(...arguments); @@ -180,9 +196,9 @@ class GoogleDriveAction extends Hub.OAuthActionV2 { type: "string", required: true, }); - const encryptedPayload = await this.oauthMaybeEncryptTokens(tokenPayload, new Hub.ActionCrypto(), request.webhookId); + const encryptedPayload = await this.oauthMaybeEncryptTokens(tokenPayload, request.webhookId); form.state = new Hub.ActionState(); - form.state.data = JSON.stringify(encryptedPayload); + form.state.data = JSON.stringify(encryptedPayload.asJson()); return form; } } @@ -248,8 +264,8 @@ class GoogleDriveAction extends Hub.OAuthActionV2 { const state = JSON.parse(plaintext); const tokens = await this.getAccessTokenCredentialsFromCode(state.redirecturi, state.code); if (this.validTokens(tokens, request.webhookId)) { - const tokenPayload = new Hub.ActionToken(tokens, state.redirecturi); - const encryptedPayload = await this.oauthMaybeEncryptTokens(tokenPayload, new Hub.ActionCrypto(), request.webhookId); + const tokenPayload = new DriveTokens(tokens, state.redirecturi); + const encryptedPayload = await this.oauthMaybeEncryptTokens(tokenPayload, request.webhookId); return encryptedPayload; } else { @@ -437,7 +453,7 @@ class GoogleDriveAction extends Hub.OAuthActionV2 { } else if (state.tokens && state.redirect) { winston.info("Extracting unencrypted state_json", { webhookId: requestWebhookId }); - tokenPayload = new Hub.ActionToken(state.tokens, state.redirect); + tokenPayload = new DriveTokens(state.tokens, state.redirect); } if (tokenPayload === null) { winston.error("Invalid state_json", { webhookId: requestWebhookId }); @@ -453,35 +469,27 @@ class GoogleDriveAction extends Hub.OAuthActionV2 { return false; } } - async oauthMaybeEncryptTokens(tokenPayload, actionCrypto, requestWebhookId) { + async oauthMaybeEncryptTokens(tokenPayload, requestWebhookId) { if (process.env.ENCRYPT_PAYLOAD === "true") { - return this.oauthEncryptTokens(tokenPayload, actionCrypto, requestWebhookId); + return Hub.EncryptedPayload.encrypt(tokenPayload, requestWebhookId); } else { return tokenPayload; } } - async oauthEncryptTokens(tokenPayload, actionCrypto, requestWebhookId) { - const jsonPayload = JSON.stringify(tokenPayload); - const encrypted = await actionCrypto.encrypt(jsonPayload).catch((err) => { - winston.error("Encryption not correctly configured", { webhookId: requestWebhookId }); - throw err; - }); - return new Hub.EncryptedPayload(actionCrypto.cipherId(), encrypted); - } async oauthDecryptTokens(encryptedPayload, actionCrypto, requestWebhookId) { const jsonPayload = await actionCrypto.decrypt(encryptedPayload.payload).catch((err) => { winston.error("Failed to decrypt state_json", { webhookId: requestWebhookId }); throw err; }); - const tokenPayload = JSON.parse(jsonPayload); + const tokenPayload = DriveTokens.fromJson(JSON.parse(jsonPayload)); return tokenPayload; } async oauthFetchAndStoreInfo(urlParams, redirectUri, statePayload, requestWebhookId) { const tokens = await this.getAccessTokenCredentialsFromCode(redirectUri, urlParams.code); if (this.validTokens(tokens, requestWebhookId)) { - const tokenPayload = new Hub.ActionToken(tokens, redirectUri); - const encryptedPayload = await this.oauthMaybeEncryptTokens(tokenPayload, new Hub.ActionCrypto(), requestWebhookId); + const tokenPayload = new DriveTokens(tokens, redirectUri); + const encryptedPayload = await this.oauthMaybeEncryptTokens(tokenPayload, requestWebhookId); await https.post({ url: statePayload.stateurl, body: JSON.stringify(encryptedPayload), diff --git a/lib/crypto/aes_transit_crypto.js b/lib/crypto/aes_transit_crypto.js index 647e63c66..288b72df8 100644 --- a/lib/crypto/aes_transit_crypto.js +++ b/lib/crypto/aes_transit_crypto.js @@ -11,7 +11,7 @@ class AESTransitCrypto { } async encrypt(plaintext) { if (process.env.CIPHER_MASTER === undefined) { - throw "CIPHER_MASTER environment variable not set"; + throw new Error("CIPHER_MASTER environment variable not set"); } const masterbuffer = Buffer.from(process.env.CIPHER_MASTER, "hex"); const dataKey = crypto.randomBytes(32); @@ -38,7 +38,7 @@ class AESTransitCrypto { } async decrypt(ciphertext) { if (process.env.CIPHER_MASTER === undefined) { - throw "CIPHER_MASTER environment variable not set"; + throw new Error("CIPHER_MASTER environment variable not set"); } const masterBuffer = Buffer.from(process.env.CIPHER_MASTER, "hex"); const keySize = Number(ciphertext.substring(1, 4)); @@ -58,7 +58,7 @@ class AESTransitCrypto { } cipherId() { if (process.env.CIPHER_MASTER === undefined) { - throw "CIPHER_MASTER environment variable not set"; + throw new Error("CIPHER_MASTER environment variable not set"); } const masterBuffer = Buffer.from(process.env.CIPHER_MASTER, "hex"); const hash = crypto.createHash(this.HASH_ALGORITHM); diff --git a/lib/hub/action_token.d.ts b/lib/hub/action_token.d.ts deleted file mode 100644 index 932788a3c..000000000 --- a/lib/hub/action_token.d.ts +++ /dev/null @@ -1,6 +0,0 @@ -export declare class ActionToken { - tokens: any; - redirect: any; - constructor(tokens: any, redirect: any); - asJson(): any; -} diff --git a/lib/hub/action_token.js b/lib/hub/action_token.js deleted file mode 100644 index 611096061..000000000 --- a/lib/hub/action_token.js +++ /dev/null @@ -1,16 +0,0 @@ -"use strict"; -Object.defineProperty(exports, "__esModule", { value: true }); -exports.ActionToken = void 0; -class ActionToken { - constructor(tokens, redirect) { - this.tokens = tokens; - this.redirect = redirect; - } - asJson() { - return { - tokens: this.tokens, - redirect: this.redirect, - }; - } -} -exports.ActionToken = ActionToken; diff --git a/lib/hub/encrypted_payload.d.ts b/lib/hub/encrypted_payload.d.ts index afcfc6aa5..a17b04cbc 100644 --- a/lib/hub/encrypted_payload.d.ts +++ b/lib/hub/encrypted_payload.d.ts @@ -1,6 +1,14 @@ +import { TokenPayload } from "."; +import { ActionCrypto } from "."; export declare class EncryptedPayload { cid: string; payload: string; + private static _crypto; + private static _currentCipherId; + static get crypto(): ActionCrypto; + static get currentCipherId(): string; constructor(cid: string, payload: string); + static encrypt(tokenPayload: TokenPayload, webhookId: string | undefined): Promise; + decrypt(webhookId: string | undefined): Promise; asJson(): any; } diff --git a/lib/hub/encrypted_payload.js b/lib/hub/encrypted_payload.js index 3132099cf..e3852f068 100644 --- a/lib/hub/encrypted_payload.js +++ b/lib/hub/encrypted_payload.js @@ -1,11 +1,41 @@ "use strict"; Object.defineProperty(exports, "__esModule", { value: true }); exports.EncryptedPayload = void 0; +const _1 = require("."); +const winston = require("winston"); class EncryptedPayload { + static get crypto() { + if (this._crypto === undefined) { + this._crypto = new _1.ActionCrypto(); + } + return this._crypto; + } + static get currentCipherId() { + if (this._currentCipherId === undefined) { + this._currentCipherId = this.crypto.cipherId(); + } + return this._currentCipherId; + } constructor(cid, payload) { this.cid = cid; this.payload = payload; } + static async encrypt(tokenPayload, webhookId) { + const jsonPayload = JSON.stringify(tokenPayload.asJson()); + const encrypted = await this.crypto.encrypt(jsonPayload).catch((err) => { + winston.error("Encryption not correctly configured", { webhookId: webhookId }); + throw err; + }); + return new EncryptedPayload(this.currentCipherId, encrypted); + } + async decrypt(webhookId) { + const jsonPayload = await EncryptedPayload.crypto.decrypt(this.payload).catch((err) => { + winston.error("Failed to decrypt state_json", { webhookId: webhookId }); + throw err; + }); + const tokenPayload = JSON.parse(jsonPayload); + return tokenPayload; + } asJson() { return { cid: this.cid, @@ -14,3 +44,5 @@ class EncryptedPayload { } } exports.EncryptedPayload = EncryptedPayload; +EncryptedPayload._crypto = undefined; +EncryptedPayload._currentCipherId = undefined; diff --git a/lib/hub/index.d.ts b/lib/hub/index.d.ts index 0cf0eae8e..abe0f3e01 100644 --- a/lib/hub/index.d.ts +++ b/lib/hub/index.d.ts @@ -6,13 +6,13 @@ export * from "./action"; export * from "./oauth_action"; export * from "./oauth_action_v2"; export * from "./delegate_oauth_action"; -export * from "./action_token"; -export * from "./encrypted_payload"; +export * from "./token_payload"; export * from "./sources"; export * from "./utils"; import { LookmlModelExploreField as FieldBase } from "../api_types/lookml_model_explore_field"; import { LookmlModelExploreFieldset as ExploreFieldset } from "../api_types/lookml_model_explore_fieldset"; import { AESTransitCrypto as ActionCrypto } from "../crypto/aes_transit_crypto"; +export * from "./encrypted_payload"; import * as JsonDetail from "./json_detail"; interface Field extends Partial { name: string; diff --git a/lib/hub/index.js b/lib/hub/index.js index 2e90b4e0e..558a27862 100644 --- a/lib/hub/index.js +++ b/lib/hub/index.js @@ -24,12 +24,12 @@ __exportStar(require("./action"), exports); __exportStar(require("./oauth_action"), exports); __exportStar(require("./oauth_action_v2"), exports); __exportStar(require("./delegate_oauth_action"), exports); -__exportStar(require("./action_token"), exports); -__exportStar(require("./encrypted_payload"), exports); +__exportStar(require("./token_payload"), exports); __exportStar(require("./sources"), exports); __exportStar(require("./utils"), exports); const aes_transit_crypto_1 = require("../crypto/aes_transit_crypto"); Object.defineProperty(exports, "ActionCrypto", { enumerable: true, get: function () { return aes_transit_crypto_1.AESTransitCrypto; } }); +__exportStar(require("./encrypted_payload"), exports); const JsonDetail = require("./json_detail"); exports.JsonDetail = JsonDetail; function allFields(fields) { diff --git a/lib/hub/oauth_action_v2.d.ts b/lib/hub/oauth_action_v2.d.ts index 27c1ea16f..d85e4c9e3 100644 --- a/lib/hub/oauth_action_v2.d.ts +++ b/lib/hub/oauth_action_v2.d.ts @@ -1,14 +1,14 @@ import { Action, RouteBuilder } from "./action"; import { ActionRequest } from "./action_request"; -import { ActionToken } from "./action_token"; import { EncryptedPayload } from "./encrypted_payload"; +import { TokenPayload } from "./token_payload"; export declare abstract class OAuthActionV2 extends Action { abstract oauthCheck(request: ActionRequest): Promise; abstract oauthUrl(redirectUri: string, encryptedState: string): Promise; abstract oauthHandleRedirect(urlParams: { [key: string]: string; }, redirectUri: string): Promise; - abstract oauthFetchAccessToken(request: ActionRequest): Promise; + abstract oauthFetchAccessToken(request: ActionRequest): Promise; asJson(router: RouteBuilder, request: ActionRequest): any; } export declare function isOauthActionV2(action: Action): boolean; diff --git a/lib/hub/token_payload.d.ts b/lib/hub/token_payload.d.ts new file mode 100644 index 000000000..613beaee7 --- /dev/null +++ b/lib/hub/token_payload.d.ts @@ -0,0 +1,4 @@ +export declare abstract class TokenPayload { + static fromJson(_json: any): TokenPayload; + abstract asJson(): any; +} diff --git a/lib/hub/token_payload.js b/lib/hub/token_payload.js new file mode 100644 index 000000000..43c7c16eb --- /dev/null +++ b/lib/hub/token_payload.js @@ -0,0 +1,9 @@ +"use strict"; +Object.defineProperty(exports, "__esModule", { value: true }); +exports.TokenPayload = void 0; +class TokenPayload { + static fromJson(_json) { + throw new Error("Not implemented: fromJson"); + } +} +exports.TokenPayload = TokenPayload; diff --git a/src/actions/google/drive/drive_tokens.ts b/src/actions/google/drive/drive_tokens.ts new file mode 100644 index 000000000..7ec68d991 --- /dev/null +++ b/src/actions/google/drive/drive_tokens.ts @@ -0,0 +1,19 @@ +import { TokenPayload } from "../../../hub" + +export class DriveTokens extends TokenPayload { + + static fromJson(json: any): DriveTokens { + return new DriveTokens(json.tokens, json.redirect) + } + + constructor(public tokens: any, public redirect: string) { + super() + } + + asJson(): any { + return { + tokens: this.tokens, + redirect: this.redirect, + } + } +} diff --git a/src/actions/google/drive/google_drive.ts b/src/actions/google/drive/google_drive.ts index 89027b4e1..674f1dc21 100644 --- a/src/actions/google/drive/google_drive.ts +++ b/src/actions/google/drive/google_drive.ts @@ -12,6 +12,7 @@ import { Error, errorWith } from "../../../hub/action_response" import Drive = drive_v3.Drive import { DomainValidator } from "./domain_validator" +import { DriveTokens } from "./drive_tokens" const LOG_PREFIX = "[GOOGLE_DRIVE]" const FOLDERID_REGEX = /\/folders\/(?[^\/?]+)/ @@ -232,11 +233,10 @@ export class GoogleDriveAction extends Hub.OAuthActionV2 { }) const encryptedPayload = await this.oauthMaybeEncryptTokens( tokenPayload, - new Hub.ActionCrypto(), request.webhookId, ) form.state = new Hub.ActionState() - form.state.data = JSON.stringify(encryptedPayload) + form.state.data = JSON.stringify(encryptedPayload.asJson()) return form } } catch (e: any) { @@ -310,10 +310,9 @@ export class GoogleDriveAction extends Hub.OAuthActionV2 { const tokens = await this.getAccessTokenCredentialsFromCode(state.redirecturi, state.code) if (this.validTokens(tokens, request.webhookId)) { - const tokenPayload = new Hub.ActionToken(tokens, state.redirecturi) + const tokenPayload = new DriveTokens(tokens, state.redirecturi) const encryptedPayload = await this.oauthMaybeEncryptTokens( tokenPayload, - new Hub.ActionCrypto(), request.webhookId, ) return encryptedPayload @@ -506,42 +505,42 @@ export class GoogleDriveAction extends Hub.OAuthActionV2 { protected async oauthExtractTokensFromStateJson( stateJson: string, requestWebhookId: string | undefined, - ): Promise { - let state: any + ): Promise { + let state: any + try { + state = JSON.parse(stateJson) + } catch (e: any) { + winston.error( + `Failed to parse state_json`, + { webhookId: requestWebhookId }, + ) + return null + } + let tokenPayload: DriveTokens | null = null + if (state.cid && state.payload) { + winston.info("Extracting encrypted state_json", { webhookId: requestWebhookId }) + const encryptedPayload = new Hub.EncryptedPayload(state.cid, state.payload) try { - state = JSON.parse(stateJson) + tokenPayload = await this.oauthDecryptTokens( + encryptedPayload, + new Hub.ActionCrypto(), + requestWebhookId, + ) } catch (e: any) { winston.error( - `Failed to parse state_json`, - {webhookId: requestWebhookId}, + `Failed to decrypt or parse encrypted payload: ${e.message}`, + { webhookId: requestWebhookId }, ) - return null + // tokenPayload remains null } - let tokenPayload: Hub.ActionToken | null = null - if (state.cid && state.payload) { - winston.info("Extracting encrypted state_json", {webhookId: requestWebhookId}) - const encryptedPayload = new Hub.EncryptedPayload(state.cid, state.payload) - try { - tokenPayload = await this.oauthDecryptTokens( - encryptedPayload, - new Hub.ActionCrypto(), - requestWebhookId, - ) - } catch (e: any) { - winston.error( - `Failed to decrypt or parse encrypted payload: ${e.message}`, - {webhookId: requestWebhookId}, - ) - // tokenPayload remains null - } - } else if (state.tokens && state.redirect) { - winston.info("Extracting unencrypted state_json", {webhookId: requestWebhookId}) - tokenPayload = new Hub.ActionToken(state.tokens, state.redirect) - } - if (tokenPayload === null) { - winston.error("Invalid state_json", {webhookId: requestWebhookId}) - } - return tokenPayload + } else if (state.tokens && state.redirect) { + winston.info("Extracting unencrypted state_json", { webhookId: requestWebhookId }) + tokenPayload = new DriveTokens(state.tokens, state.redirect) + } + if (tokenPayload === null) { + winston.error("Invalid state_json", { webhookId: requestWebhookId }) + } + return tokenPayload } protected validTokens( @@ -557,40 +556,27 @@ export class GoogleDriveAction extends Hub.OAuthActionV2 { } protected async oauthMaybeEncryptTokens( - tokenPayload: Hub.ActionToken, - actionCrypto: Hub.ActionCrypto, + tokenPayload: DriveTokens, requestWebhookId: string | undefined, - ): Promise { + ): Promise { if (process.env.ENCRYPT_PAYLOAD === "true") { - return this.oauthEncryptTokens(tokenPayload, actionCrypto, requestWebhookId) + return Hub.EncryptedPayload.encrypt(tokenPayload, requestWebhookId) } else { return tokenPayload } } - protected async oauthEncryptTokens( - tokenPayload: Hub.ActionToken, - actionCrypto: Hub.ActionCrypto, - requestWebhookId: string | undefined, - ): Promise { - const jsonPayload = JSON.stringify(tokenPayload) - const encrypted = await actionCrypto.encrypt(jsonPayload).catch((err: string) => { - winston.error("Encryption not correctly configured", {webhookId: requestWebhookId}) - throw err - }) - return new Hub.EncryptedPayload(actionCrypto.cipherId(), encrypted) - } - protected async oauthDecryptTokens( encryptedPayload: Hub.EncryptedPayload, actionCrypto: Hub.ActionCrypto, requestWebhookId: string | undefined, - ): Promise { + ): Promise { const jsonPayload = await actionCrypto.decrypt(encryptedPayload.payload).catch((err: string) => { winston.error("Failed to decrypt state_json", {webhookId: requestWebhookId}) throw err }) - const tokenPayload: Hub.ActionToken = JSON.parse(jsonPayload) + + const tokenPayload = DriveTokens.fromJson(JSON.parse(jsonPayload)) return tokenPayload } @@ -602,10 +588,9 @@ export class GoogleDriveAction extends Hub.OAuthActionV2 { ) { const tokens = await this.getAccessTokenCredentialsFromCode(redirectUri, urlParams.code) if (this.validTokens(tokens, requestWebhookId)) { - const tokenPayload = new Hub.ActionToken(tokens, redirectUri) + const tokenPayload = new DriveTokens(tokens, redirectUri) const encryptedPayload = await this.oauthMaybeEncryptTokens( tokenPayload, - new Hub.ActionCrypto(), requestWebhookId, ) await https.post({ diff --git a/src/crypto/aes_transit_crypto.ts b/src/crypto/aes_transit_crypto.ts index 444ba6b4c..e870b899f 100644 --- a/src/crypto/aes_transit_crypto.ts +++ b/src/crypto/aes_transit_crypto.ts @@ -9,7 +9,7 @@ export class AESTransitCrypto implements CryptoProvider { async encrypt(plaintext: string) { if (process.env.CIPHER_MASTER === undefined) { - throw "CIPHER_MASTER environment variable not set" + throw new Error("CIPHER_MASTER environment variable not set") } const masterbuffer = Buffer.from(process.env.CIPHER_MASTER, "hex") const dataKey = crypto.randomBytes(32) @@ -40,7 +40,7 @@ export class AESTransitCrypto implements CryptoProvider { } async decrypt(ciphertext: string) { if (process.env.CIPHER_MASTER === undefined) { - throw "CIPHER_MASTER environment variable not set" + throw new Error("CIPHER_MASTER environment variable not set") } const masterBuffer = Buffer.from(process.env.CIPHER_MASTER, "hex") const keySize = Number(ciphertext.substring(1, 4)) @@ -63,7 +63,7 @@ export class AESTransitCrypto implements CryptoProvider { cipherId() { if (process.env.CIPHER_MASTER === undefined) { - throw "CIPHER_MASTER environment variable not set" + throw new Error("CIPHER_MASTER environment variable not set") } const masterBuffer = Buffer.from(process.env.CIPHER_MASTER, "hex") diff --git a/src/hub/action_token.ts b/src/hub/action_token.ts deleted file mode 100644 index b8ef0a751..000000000 --- a/src/hub/action_token.ts +++ /dev/null @@ -1,10 +0,0 @@ -export class ActionToken { - constructor(public tokens: any, public redirect: any) {} - - asJson(): any { - return { - tokens: this.tokens, - redirect: this.redirect, - } - } -} diff --git a/src/hub/encrypted_payload.ts b/src/hub/encrypted_payload.ts index 9b63730ce..69ed196af 100644 --- a/src/hub/encrypted_payload.ts +++ b/src/hub/encrypted_payload.ts @@ -1,6 +1,49 @@ +import { TokenPayload } from "." +import { ActionCrypto } from "." + +import * as winston from "winston" + export class EncryptedPayload { + + static get crypto() { + if (this._crypto === undefined) { + this._crypto = new ActionCrypto() + } + return this._crypto + } + + static get currentCipherId() { + if (this._currentCipherId === undefined) { + this._currentCipherId = this.crypto.cipherId() + } + return this._currentCipherId + } + + static async encrypt( + tokenPayload: TokenPayload, + webhookId: string | undefined, + ): Promise { + const jsonPayload = JSON.stringify(tokenPayload.asJson()) + const encrypted = await this.crypto.encrypt(jsonPayload).catch((err: string) => { + winston.error("Encryption not correctly configured", { webhookId }) + throw err + }) + return new EncryptedPayload(this.currentCipherId, encrypted) + } + private static _crypto: ActionCrypto | undefined = undefined + private static _currentCipherId: string | undefined = undefined + constructor(public cid: string, public payload: string) {} + async decrypt(webhookId: string | undefined): Promise { + const jsonPayload = await EncryptedPayload.crypto.decrypt(this.payload).catch((err: string) => { + winston.error("Failed to decrypt state_json", { webhookId }) + throw err + }) + const tokenPayload: TokenPayload = JSON.parse(jsonPayload) + return tokenPayload + } + asJson(): any { return { cid: this.cid, diff --git a/src/hub/index.ts b/src/hub/index.ts index 751acf6de..914705ab3 100644 --- a/src/hub/index.ts +++ b/src/hub/index.ts @@ -6,14 +6,14 @@ export * from "./action" export * from "./oauth_action" export * from "./oauth_action_v2" export * from "./delegate_oauth_action" -export * from "./action_token" -export * from "./encrypted_payload" +export * from "./token_payload" export * from "./sources" export * from "./utils" import { LookmlModelExploreField as FieldBase } from "../api_types/lookml_model_explore_field" import { LookmlModelExploreFieldset as ExploreFieldset } from "../api_types/lookml_model_explore_fieldset" import { AESTransitCrypto as ActionCrypto } from "../crypto/aes_transit_crypto" +export * from "./encrypted_payload" import * as JsonDetail from "./json_detail" diff --git a/src/hub/oauth_action_v2.ts b/src/hub/oauth_action_v2.ts index 3c15cfc6d..a7508bb20 100644 --- a/src/hub/oauth_action_v2.ts +++ b/src/hub/oauth_action_v2.ts @@ -1,13 +1,13 @@ import {Action, RouteBuilder} from "./action" -import {ActionRequest} from "./action_request" -import {ActionToken} from "./action_token" +import { ActionRequest } from "./action_request" import {EncryptedPayload} from "./encrypted_payload" +import { TokenPayload } from "./token_payload" export abstract class OAuthActionV2 extends Action { abstract oauthCheck(request: ActionRequest): Promise abstract oauthUrl(redirectUri: string, encryptedState: string): Promise abstract oauthHandleRedirect(urlParams: { [key: string]: string }, redirectUri: string): Promise - abstract oauthFetchAccessToken(request: ActionRequest): Promise + abstract oauthFetchAccessToken(request: ActionRequest): Promise asJson(router: RouteBuilder, request: ActionRequest): any { const json = super.asJson(router, request) diff --git a/src/hub/token_payload.ts b/src/hub/token_payload.ts new file mode 100644 index 000000000..7a496d5fc --- /dev/null +++ b/src/hub/token_payload.ts @@ -0,0 +1,7 @@ +export abstract class TokenPayload { + static fromJson(_json: any): TokenPayload { + throw new Error("Not implemented: fromJson") + } + + abstract asJson(): any +} From d71aaebedb006396cd02c971b1687f4a77c147b6 Mon Sep 17 00:00:00 2001 From: Brian Graves Date: Thu, 12 Feb 2026 14:35:06 -0800 Subject: [PATCH 2/4] WIP applying fix to remaining oauth actions --- lib/actions/airtable/airtable.js | 31 +++-- lib/actions/dropbox/dropbox.d.ts | 2 +- lib/actions/dropbox/dropbox.js | 22 ++-- .../facebook/facebook_custom_audiences.js | 7 +- lib/actions/google/ads/customer_match.js | 6 +- lib/actions/google/ads/lib/ads_request.d.ts | 2 +- lib/actions/google/ads/lib/ads_request.js | 8 +- lib/actions/google/analytics/data_import.js | 3 +- .../google/analytics/lib/ga_worker.d.ts | 2 +- lib/actions/google/analytics/lib/ga_worker.js | 8 +- lib/actions/google/common/oauth_helper.js | 3 +- lib/actions/google/common/utils.d.ts | 2 - lib/actions/google/common/utils.js | 17 --- lib/actions/google/common/wrapped_response.js | 2 +- lib/actions/google/drive/drive_tokens.d.ts | 8 ++ lib/actions/google/drive/drive_tokens.js | 21 ++++ lib/actions/google/drive/google_drive.d.ts | 8 +- lib/actions/google/drive/google_drive.js | 25 +--- .../campaigns/salesforce_campaigns.js | 13 ++- lib/hub/encrypted_payload.d.ts | 4 +- lib/hub/encrypted_payload.js | 24 ++-- lib/hub/oauth_action.d.ts | 4 + lib/hub/oauth_action.js | 56 +++++++++ src/actions/airtable/airtable.ts | 31 +++-- src/actions/dropbox/dropbox.ts | 22 ++-- .../facebook/facebook_custom_audiences.ts | 7 +- .../test_facebook_custom_audiences.ts | 2 +- src/actions/google/ads/customer_match.ts | 6 +- src/actions/google/ads/lib/ads_request.ts | 11 +- src/actions/google/ads/test_customer_match.ts | 2 +- src/actions/google/analytics/data_import.ts | 3 +- src/actions/google/analytics/lib/ga_worker.ts | 11 +- .../google/analytics/test_data_import.ts | 2 +- src/actions/google/common/oauth_helper.ts | 3 +- src/actions/google/common/wrapped_response.ts | 2 +- .../campaigns/salesforce_campaigns.ts | 13 ++- src/hub/encrypted_payload.ts | 13 +-- src/hub/oauth_action.ts | 79 +++++++++++++ test/test.ts | 1 + test/test_oauth_action.ts | 107 ++++++++++++++++++ 40 files changed, 430 insertions(+), 163 deletions(-) delete mode 100644 lib/actions/google/common/utils.d.ts delete mode 100644 lib/actions/google/common/utils.js create mode 100644 lib/actions/google/drive/drive_tokens.d.ts create mode 100644 lib/actions/google/drive/drive_tokens.js create mode 100644 test/test_oauth_action.ts diff --git a/lib/actions/airtable/airtable.js b/lib/actions/airtable/airtable.js index a8a504101..ef2f19ea3 100644 --- a/lib/actions/airtable/airtable.js +++ b/lib/actions/airtable/airtable.js @@ -50,31 +50,33 @@ class AirtableAction extends Hub.OAuthAction { try { let accessToken; if (request.params.state_json) { - const stateJson = JSON.parse(request.params.state_json); + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); accessToken = stateJson.tokens.access_token; - state.data = JSON.stringify({ + const encrypted = await this.oauthMaybeEncryptTokens({ tokens: { refresh_token: stateJson.tokens.refresh_token, access_token: accessToken, }, - }); + }, request.webhookId); + state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); } try { await this.executeAirtable(request, records, accessToken); } catch (_a) { if (request.params.state_json) { - const stateJson = JSON.parse(request.params.state_json); + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); const refreshResponse = await this.refreshTokens(stateJson.tokens.refresh_token); // @ts-ignore if (Object.keys(refreshResponse.data).length !== 0) { accessToken = refreshResponse.data.access_token; - state.data = JSON.stringify({ + const encrypted = await this.oauthMaybeEncryptTokens({ tokens: { refresh_token: refreshResponse.data.refresh_token, access_token: accessToken, }, - }); + }, request.webhookId); + state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); } else { delete state.data; @@ -106,30 +108,37 @@ class AirtableAction extends Hub.OAuthAction { const form = new Hub.ActionForm(); try { let accessToken; + let stateJson; if (request.params.state_json) { - const stateJson = JSON.parse(request.params.state_json); + stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); accessToken = stateJson.tokens.access_token; } try { await this.checkBaseList(accessToken); if (form.state === undefined) { form.state = new hub_1.ActionState(); - form.state.data = request.params.state_json; + if (stateJson) { + const encrypted = await this.oauthMaybeEncryptTokens(stateJson, request.webhookId); + form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); + } } } catch (_a) { // Assume the failure is due to Oauth failure, // refresh token and retry once. if (request.params.state_json) { - const stateJson = JSON.parse(request.params.state_json); + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); const refreshResponse = await this.refreshTokens(stateJson.tokens.refresh_token); if (Object.keys(refreshResponse.data).length !== 0) { accessToken = refreshResponse.data.access_token; form.state = new hub_1.ActionState(); - form.state.data = JSON.stringify({ tokens: { + const encrypted = await this.oauthMaybeEncryptTokens({ + tokens: { refresh_token: refreshResponse.data.refresh_token, access_token: accessToken, - } }); + } + }, request.webhookId); + form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); } } await this.checkBaseList(accessToken); diff --git a/lib/actions/dropbox/dropbox.d.ts b/lib/actions/dropbox/dropbox.d.ts index 49f58e65c..4b1183433 100644 --- a/lib/actions/dropbox/dropbox.d.ts +++ b/lib/actions/dropbox/dropbox.d.ts @@ -19,5 +19,5 @@ export declare class DropboxAction extends Hub.OAuthAction { oauthCheck(request: Hub.ActionRequest): Promise; dropboxFilename(request: Hub.ActionRequest): string | undefined; protected getAccessTokenFromCode(stateJson: any): Promise; - protected dropboxClientFromRequest(request: Hub.ActionRequest, token: string): Dropbox; + protected dropboxClientFromRequest(request: Hub.ActionRequest, token: string): Promise; } diff --git a/lib/actions/dropbox/dropbox.js b/lib/actions/dropbox/dropbox.js index 45d239a68..53b072516 100644 --- a/lib/actions/dropbox/dropbox.js +++ b/lib/actions/dropbox/dropbox.js @@ -26,12 +26,12 @@ class DropboxAction extends Hub.OAuthAction { const ext = request.attachment.fileExtension; let accessToken = ""; if (request.params.state_json) { - const stateJson = JSON.parse(request.params.state_json); + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); if (stateJson.code && stateJson.redirect) { accessToken = await this.getAccessTokenFromCode(stateJson); } } - const drop = this.dropboxClientFromRequest(request, accessToken); + const drop = await this.dropboxClientFromRequest(request, accessToken); const resp = new Hub.ActionResponse(); resp.success = true; if (request.attachment && request.attachment.dataBuffer) { @@ -56,7 +56,7 @@ class DropboxAction extends Hub.OAuthAction { let accessToken = ""; if (request.params.state_json) { try { - const stateJson = JSON.parse(request.params.state_json); + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); if (stateJson.code && stateJson.redirect) { accessToken = await this.getAccessTokenFromCode(stateJson); } @@ -65,7 +65,7 @@ class DropboxAction extends Hub.OAuthAction { winston.warn("Could not parse state_json"); } } - const drop = this.dropboxClientFromRequest(request, accessToken); + const drop = await this.dropboxClientFromRequest(request, accessToken); try { const response = await drop.filesListFolder({ path: "" }); const folderList = response.entries.filter((entries) => (entries[".tag"] === "folder")) @@ -100,9 +100,11 @@ class DropboxAction extends Hub.OAuthAction { }], }]; if (accessToken !== "") { - const newState = JSON.stringify({ access_token: accessToken }); - form.state = new Hub.ActionState(); - form.state.data = newState; + if (accessToken !== "") { + const encrypted = await this.oauthMaybeEncryptTokens({ access_token: accessToken }, request.webhookId); + form.state = new Hub.ActionState(); + form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); + } } return form; } @@ -149,7 +151,7 @@ class DropboxAction extends Hub.OAuthAction { }).catch((_err) => { winston.error(_err.toString()); }); } async oauthCheck(request) { - const drop = this.dropboxClientFromRequest(request, ""); + const drop = await this.dropboxClientFromRequest(request, ""); try { await drop.filesListFolder({ path: "" }); return true; @@ -185,10 +187,10 @@ class DropboxAction extends Hub.OAuthAction { .catch((_err) => { winston.error("Error requesting access_token"); }); return response.access_token; } - dropboxClientFromRequest(request, token) { + async dropboxClientFromRequest(request, token) { if (request.params.state_json && token === "") { try { - const json = JSON.parse(request.params.state_json); + const json = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); token = json.access_token; } catch (er) { diff --git a/lib/actions/facebook/facebook_custom_audiences.js b/lib/actions/facebook/facebook_custom_audiences.js index 75a9b876d..3c8884798 100644 --- a/lib/actions/facebook/facebook_custom_audiences.js +++ b/lib/actions/facebook/facebook_custom_audiences.js @@ -97,10 +97,11 @@ class FacebookCustomAudiencesAction extends Hub.OAuthAction { const userState = { tokens, redirect: redirectUri }; // So now we use that state url to persist the oauth tokens try { + const encrypted = await this.oauthMaybeEncryptTokens(userState, payload.webhookId); await gaxios.request({ method: "POST", url: payload.stateUrl, - data: userState, + data: typeof encrypted === "string" ? encrypted : encrypted, }); } catch (err) { @@ -151,8 +152,8 @@ class FacebookCustomAudiencesAction extends Hub.OAuthAction { } async getAccessTokenFromRequest(request) { try { - const params = request.params; - return JSON.parse(params.state_json).tokens.longLivedToken; + const stateJson = await this.oauthExtractTokensFromStateJson(`${request.params.state_json}`, request.webhookId); + return stateJson.tokens.longLivedToken; } catch (e) { winston.error(`${LOG_PREFIX} Failed to parse state for access token.`, { webhookId: request.webhookId }); diff --git a/lib/actions/google/ads/customer_match.js b/lib/actions/google/ads/customer_match.js index d19ac491d..e288eb9f3 100644 --- a/lib/actions/google/ads/customer_match.js +++ b/lib/actions/google/ads/customer_match.js @@ -63,7 +63,8 @@ class GoogleAdsCustomerMatch extends Hub.OAuthAction { const adsRequest = await ads_request_1.GoogleAdsActionRequest.fromHub(hubReq, this, log); await adsRequest.execute(); log("info", "Execution complete"); - return wrappedResp.returnSuccess(adsRequest.userState); + const encrypted = await this.oauthMaybeEncryptTokens(adsRequest.userState, hubReq.webhookId); + return wrappedResp.returnSuccess(encrypted); } catch (err) { (0, error_utils_1.sanitizeError)(err); @@ -79,7 +80,8 @@ class GoogleAdsCustomerMatch extends Hub.OAuthAction { try { const adsWorker = await ads_request_1.GoogleAdsActionRequest.fromHub(hubReq, this, log); wrappedResp.form = await adsWorker.makeForm(); - return wrappedResp.returnSuccess(adsWorker.userState); + const encrypted = await this.oauthMaybeEncryptTokens(adsWorker.userState, hubReq.webhookId); + return wrappedResp.returnSuccess(encrypted); // Use this code if you need to force a state reset and redo oauth login // wrappedResp.form = await this.oauthHelper.makeLoginForm(hubReq) // wrappedResp.resetState() diff --git a/lib/actions/google/ads/lib/ads_request.d.ts b/lib/actions/google/ads/lib/ads_request.d.ts index bc050cc2c..848c8e72f 100644 --- a/lib/actions/google/ads/lib/ads_request.d.ts +++ b/lib/actions/google/ads/lib/ads_request.d.ts @@ -17,7 +17,7 @@ export declare class GoogleAdsActionRequest { formParams: any; userState: AdsUserState; webhookId?: string; - constructor(hubRequest: Hub.ActionRequest, actionInstance: GoogleAdsCustomerMatch, log: Logger); + constructor(hubRequest: Hub.ActionRequest, actionInstance: GoogleAdsCustomerMatch, log: Logger, userState: any); checkTokens(): Promise; setApiClient(): void; get accessToken(): string; diff --git a/lib/actions/google/ads/lib/ads_request.js b/lib/actions/google/ads/lib/ads_request.js index 4412fea1d..a54ef51c7 100644 --- a/lib/actions/google/ads/lib/ads_request.js +++ b/lib/actions/google/ads/lib/ads_request.js @@ -4,24 +4,24 @@ exports.GoogleAdsActionRequest = void 0; const winston = require("winston"); const missing_auth_error_1 = require("../../common/missing_auth_error"); const missing_required_params_error_1 = require("../../common/missing_required_params_error"); -const utils_1 = require("../../common/utils"); const ads_executor_1 = require("./ads_executor"); const ads_form_builder_1 = require("./ads_form_builder"); const api_client_1 = require("./api_client"); const LOG_PREFIX = "[G Ads Customer Match]"; class GoogleAdsActionRequest { static async fromHub(hubRequest, action, logger) { - const adsReq = new GoogleAdsActionRequest(hubRequest, action, logger); + const userState = await action.oauthExtractTokensFromStateJson(`${hubRequest.params.state_json}`, hubRequest.webhookId); + const adsReq = new GoogleAdsActionRequest(hubRequest, action, logger, userState); await adsReq.checkTokens(); adsReq.setApiClient(); return adsReq; } - constructor(hubRequest, actionInstance, log) { + constructor(hubRequest, actionInstance, log, userState) { this.hubRequest = hubRequest; this.actionInstance = actionInstance; this.log = log; this.streamingDownload = this.hubRequest.stream.bind(this.hubRequest); - const state = (0, utils_1.safeParseJson)(`${hubRequest.params.state_json}`); + const state = userState; if (!state || !state.tokens || !state.tokens.access_token || !state.tokens.refresh_token || !state.redirect) { winston.warn(`${LOG_PREFIX} User state was missing or did not contain oauth tokens & redirect`, { webhookId: hubRequest.webhookId }); throw new missing_auth_error_1.MissingAuthError("User state was missing or did not contain oauth tokens & redirect"); diff --git a/lib/actions/google/analytics/data_import.js b/lib/actions/google/analytics/data_import.js index 43938597f..8be9de44b 100644 --- a/lib/actions/google/analytics/data_import.js +++ b/lib/actions/google/analytics/data_import.js @@ -82,7 +82,8 @@ class GoogleAnalyticsDataImportAction extends Hub.OAuthAction { log("debug", "New upload id=", gaWorker.newUploadId); // Since the upload was successful, update the lastUsedFormParams in user state gaWorker.setLastUsedFormParams(); - wrappedResp.setUserState(gaWorker.userState); + const encrypted = await this.oauthMaybeEncryptTokens(gaWorker.userState, hubReq.webhookId); + wrappedResp.setUserState(encrypted); if (gaWorker.isDeleteOtherFiles) { currentStep = "Delete other files step"; await gaWorker.deleteOtherFiles(); diff --git a/lib/actions/google/analytics/lib/ga_worker.d.ts b/lib/actions/google/analytics/lib/ga_worker.d.ts index c469f5585..6c9d29c1e 100644 --- a/lib/actions/google/analytics/lib/ga_worker.d.ts +++ b/lib/actions/google/analytics/lib/ga_worker.d.ts @@ -17,7 +17,7 @@ export declare class GoogleAnalyticsActionWorker { userState: GAUserState; formParams: any; newUploadId?: string; - constructor(hubRequest: Hub.ActionRequest, actionInstance: GoogleAnalyticsDataImportAction, log: Logger); + constructor(hubRequest: Hub.ActionRequest, actionInstance: GoogleAnalyticsDataImportAction, log: Logger, userState: any); makeGAClient(): analytics_v3.Analytics; get redirect(): string; get tokens(): Credentials; diff --git a/lib/actions/google/analytics/lib/ga_worker.js b/lib/actions/google/analytics/lib/ga_worker.js index 34ccfff0e..271af1caf 100644 --- a/lib/actions/google/analytics/lib/ga_worker.js +++ b/lib/actions/google/analytics/lib/ga_worker.js @@ -5,18 +5,18 @@ const googleapis_1 = require("googleapis"); const Hub = require("../../../../hub"); const missing_auth_error_1 = require("../../common/missing_auth_error"); const missing_required_params_error_1 = require("../../common/missing_required_params_error"); -const utils_1 = require("../../common/utils"); const csv_header_transform_stream_1 = require("./csv_header_transform_stream"); class GoogleAnalyticsActionWorker { static async fromHubRequest(hubRequest, actionInstance, logger) { - const gaWorker = new GoogleAnalyticsActionWorker(hubRequest, actionInstance, logger); + const userState = await actionInstance.oauthExtractTokensFromStateJson(`${hubRequest.params.state_json}`, hubRequest.webhookId); + const gaWorker = new GoogleAnalyticsActionWorker(hubRequest, actionInstance, logger, userState); return gaWorker; } - constructor(hubRequest, actionInstance, log) { + constructor(hubRequest, actionInstance, log, userState) { this.hubRequest = hubRequest; this.actionInstance = actionInstance; this.log = log; - const tmpState = (0, utils_1.safeParseJson)(hubRequest.params.state_json); + const tmpState = userState; if (!tmpState || !tmpState.tokens || !tmpState.redirect) { throw new missing_auth_error_1.MissingAuthError("User state was missing or did not contain tokens & redirect"); } diff --git a/lib/actions/google/common/oauth_helper.js b/lib/actions/google/common/oauth_helper.js index 8f613a58f..880c4b33a 100644 --- a/lib/actions/google/common/oauth_helper.js +++ b/lib/actions/google/common/oauth_helper.js @@ -89,10 +89,11 @@ class GoogleOAuthHelper { const userState = { tokens, redirect: redirectUri }; // So now we use that state url to persist the oauth tokens try { + const encrypted = await this.actionInstance.oauthMaybeEncryptTokens(userState, webhookId); await gaxios.request({ method: "POST", url: payload.stateUrl, - data: userState, + data: typeof encrypted === "string" ? encrypted : encrypted, }); } catch (err) { diff --git a/lib/actions/google/common/utils.d.ts b/lib/actions/google/common/utils.d.ts deleted file mode 100644 index 39a557d2e..000000000 --- a/lib/actions/google/common/utils.d.ts +++ /dev/null @@ -1,2 +0,0 @@ -export declare function safeParseJson(str: string | undefined): any; -export declare function isMochaRunning(): boolean; diff --git a/lib/actions/google/common/utils.js b/lib/actions/google/common/utils.js deleted file mode 100644 index e9e2d69a2..000000000 --- a/lib/actions/google/common/utils.js +++ /dev/null @@ -1,17 +0,0 @@ -"use strict"; -Object.defineProperty(exports, "__esModule", { value: true }); -exports.safeParseJson = safeParseJson; -exports.isMochaRunning = isMochaRunning; -function safeParseJson(str) { - try { - return JSON.parse(str ? str : ""); - } - catch (_a) { - return undefined; - } -} -function isMochaRunning() { - return ["afterEach", "after", "beforeEach", "before", "describe", "it"].every((functionName) => { - return global[functionName] instanceof Function; - }); -} diff --git a/lib/actions/google/common/wrapped_response.js b/lib/actions/google/common/wrapped_response.js index a94ffa8c6..142965526 100644 --- a/lib/actions/google/common/wrapped_response.js +++ b/lib/actions/google/common/wrapped_response.js @@ -40,7 +40,7 @@ class WrappedResponse { } setUserState(userState) { this._hubResp.state = new Hub.ActionState(); - this._hubResp.state.data = JSON.stringify(userState); + this._hubResp.state.data = typeof userState === "string" ? userState : JSON.stringify(userState); } } exports.WrappedResponse = WrappedResponse; diff --git a/lib/actions/google/drive/drive_tokens.d.ts b/lib/actions/google/drive/drive_tokens.d.ts new file mode 100644 index 000000000..b1d13d48a --- /dev/null +++ b/lib/actions/google/drive/drive_tokens.d.ts @@ -0,0 +1,8 @@ +import { TokenPayload } from "../../../hub"; +export declare class DriveTokens extends TokenPayload { + tokens: any; + redirect: string; + static fromJson(json: any): DriveTokens; + constructor(tokens: any, redirect: string); + asJson(): any; +} diff --git a/lib/actions/google/drive/drive_tokens.js b/lib/actions/google/drive/drive_tokens.js new file mode 100644 index 000000000..cc6163f87 --- /dev/null +++ b/lib/actions/google/drive/drive_tokens.js @@ -0,0 +1,21 @@ +"use strict"; +Object.defineProperty(exports, "__esModule", { value: true }); +exports.DriveTokens = void 0; +const hub_1 = require("../../../hub"); +class DriveTokens extends hub_1.TokenPayload { + static fromJson(json) { + return new DriveTokens(json.tokens, json.redirect); + } + constructor(tokens, redirect) { + super(); + this.tokens = tokens; + this.redirect = redirect; + } + asJson() { + return { + tokens: this.tokens, + redirect: this.redirect, + }; + } +} +exports.DriveTokens = DriveTokens; diff --git a/lib/actions/google/drive/google_drive.d.ts b/lib/actions/google/drive/google_drive.d.ts index bfd978843..54843e8ff 100644 --- a/lib/actions/google/drive/google_drive.d.ts +++ b/lib/actions/google/drive/google_drive.d.ts @@ -3,17 +3,11 @@ import { Credentials, OAuth2Client } from "google-auth-library"; import { drive_v3 } from "googleapis"; import * as Hub from "../../../hub"; import Drive = drive_v3.Drive; +import { DriveTokens } from "./drive_tokens"; interface OauthState { tokenurl?: string; stateurl?: string; } -declare class DriveTokens extends Hub.TokenPayload { - tokens: any; - redirect: string; - constructor(tokens: any, redirect: string); - static fromJson(json: any): DriveTokens; - asJson(): any; -} export declare class GoogleDriveAction extends Hub.OAuthActionV2 { name: string; label: string; diff --git a/lib/actions/google/drive/google_drive.js b/lib/actions/google/drive/google_drive.js index f02cdfabb..26ddc5ea1 100644 --- a/lib/actions/google/drive/google_drive.js +++ b/lib/actions/google/drive/google_drive.js @@ -9,24 +9,9 @@ const utils_1 = require("../../../error_types/utils"); const Hub = require("../../../hub"); const action_response_1 = require("../../../hub/action_response"); const domain_validator_1 = require("./domain_validator"); +const drive_tokens_1 = require("./drive_tokens"); const LOG_PREFIX = "[GOOGLE_DRIVE]"; const FOLDERID_REGEX = /\/folders\/(?[^\/?]+)/; -class DriveTokens extends Hub.TokenPayload { - constructor(tokens, redirect) { - super(); - this.tokens = tokens; - this.redirect = redirect; - } - static fromJson(json) { - return new DriveTokens(json.tokens, json.redirect); - } - asJson() { - return { - tokens: this.tokens, - redirect: this.redirect, - }; - } -} class GoogleDriveAction extends Hub.OAuthActionV2 { constructor() { super(...arguments); @@ -264,7 +249,7 @@ class GoogleDriveAction extends Hub.OAuthActionV2 { const state = JSON.parse(plaintext); const tokens = await this.getAccessTokenCredentialsFromCode(state.redirecturi, state.code); if (this.validTokens(tokens, request.webhookId)) { - const tokenPayload = new DriveTokens(tokens, state.redirecturi); + const tokenPayload = new drive_tokens_1.DriveTokens(tokens, state.redirecturi); const encryptedPayload = await this.oauthMaybeEncryptTokens(tokenPayload, request.webhookId); return encryptedPayload; } @@ -453,7 +438,7 @@ class GoogleDriveAction extends Hub.OAuthActionV2 { } else if (state.tokens && state.redirect) { winston.info("Extracting unencrypted state_json", { webhookId: requestWebhookId }); - tokenPayload = new DriveTokens(state.tokens, state.redirect); + tokenPayload = new drive_tokens_1.DriveTokens(state.tokens, state.redirect); } if (tokenPayload === null) { winston.error("Invalid state_json", { webhookId: requestWebhookId }); @@ -482,13 +467,13 @@ class GoogleDriveAction extends Hub.OAuthActionV2 { winston.error("Failed to decrypt state_json", { webhookId: requestWebhookId }); throw err; }); - const tokenPayload = DriveTokens.fromJson(JSON.parse(jsonPayload)); + const tokenPayload = drive_tokens_1.DriveTokens.fromJson(JSON.parse(jsonPayload)); return tokenPayload; } async oauthFetchAndStoreInfo(urlParams, redirectUri, statePayload, requestWebhookId) { const tokens = await this.getAccessTokenCredentialsFromCode(redirectUri, urlParams.code); if (this.validTokens(tokens, requestWebhookId)) { - const tokenPayload = new DriveTokens(tokens, redirectUri); + const tokenPayload = new drive_tokens_1.DriveTokens(tokens, redirectUri); const encryptedPayload = await this.oauthMaybeEncryptTokens(tokenPayload, requestWebhookId); await https.post({ url: statePayload.stateurl, diff --git a/lib/actions/salesforce/campaigns/salesforce_campaigns.js b/lib/actions/salesforce/campaigns/salesforce_campaigns.js index 0380bec0e..7c87f6711 100644 --- a/lib/actions/salesforce/campaigns/salesforce_campaigns.js +++ b/lib/actions/salesforce/campaigns/salesforce_campaigns.js @@ -113,7 +113,7 @@ class SalesforceCampaignsAction extends Hub.OAuthAction { let response = {}; let tokens; try { - const stateJson = JSON.parse(request.params.state_json); + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); if (stateJson.access_token && stateJson.refresh_token) { tokens = stateJson; } @@ -130,7 +130,8 @@ class SalesforceCampaignsAction extends Hub.OAuthAction { response.message = message; tokens = { access_token: sfdcConn.accessToken, refresh_token: sfdcConn.refreshToken }; response.state = new Hub.ActionState(); - response.state.data = JSON.stringify(tokens); + const encrypted = await this.oauthMaybeEncryptTokens(tokens, request.webhookId); + response.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); } catch (e) { response = { success: false, message: e.message }; @@ -154,21 +155,23 @@ class SalesforceCampaignsAction extends Hub.OAuthAction { // scenarios 1 and 2 will show loginForm, 3 and 4 will show formBuilder if (request.params.state_json) { try { - const stateJson = JSON.parse(request.params.state_json); + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); if (stateJson.access_token && stateJson.refresh_token) { tokens = stateJson; } else { tokens = await this.sfdcOauthHelper.getAccessTokensFromAuthCode(stateJson); form.state = new Hub.ActionState(); - form.state.data = JSON.stringify(tokens); + const encrypted = await this.oauthMaybeEncryptTokens(tokens, request.webhookId); + form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); } // passing back connection object to handle access token refresh and update state const { fields, sfdcConn } = await this.sfdcCampaignsFormBuilder.formBuilder(request, tokens); form.fields = fields; tokens = { access_token: sfdcConn.accessToken, refresh_token: sfdcConn.refreshToken }; form.state = new Hub.ActionState(); - form.state.data = JSON.stringify(tokens); + const encrypted = await this.oauthMaybeEncryptTokens(tokens, request.webhookId); + form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); return form; } catch (e) { diff --git a/lib/hub/encrypted_payload.d.ts b/lib/hub/encrypted_payload.d.ts index a17b04cbc..c1c16c0ec 100644 --- a/lib/hub/encrypted_payload.d.ts +++ b/lib/hub/encrypted_payload.d.ts @@ -3,12 +3,10 @@ import { ActionCrypto } from "."; export declare class EncryptedPayload { cid: string; payload: string; - private static _crypto; - private static _currentCipherId; static get crypto(): ActionCrypto; static get currentCipherId(): string; - constructor(cid: string, payload: string); static encrypt(tokenPayload: TokenPayload, webhookId: string | undefined): Promise; + constructor(cid: string, payload: string); decrypt(webhookId: string | undefined): Promise; asJson(): any; } diff --git a/lib/hub/encrypted_payload.js b/lib/hub/encrypted_payload.js index e3852f068..75e76d85f 100644 --- a/lib/hub/encrypted_payload.js +++ b/lib/hub/encrypted_payload.js @@ -5,32 +5,26 @@ const _1 = require("."); const winston = require("winston"); class EncryptedPayload { static get crypto() { - if (this._crypto === undefined) { - this._crypto = new _1.ActionCrypto(); - } - return this._crypto; + return new _1.ActionCrypto(); } static get currentCipherId() { - if (this._currentCipherId === undefined) { - this._currentCipherId = this.crypto.cipherId(); - } - return this._currentCipherId; - } - constructor(cid, payload) { - this.cid = cid; - this.payload = payload; + return this.crypto.cipherId(); } static async encrypt(tokenPayload, webhookId) { const jsonPayload = JSON.stringify(tokenPayload.asJson()); const encrypted = await this.crypto.encrypt(jsonPayload).catch((err) => { - winston.error("Encryption not correctly configured", { webhookId: webhookId }); + winston.error("Encryption not correctly configured", { webhookId }); throw err; }); return new EncryptedPayload(this.currentCipherId, encrypted); } + constructor(cid, payload) { + this.cid = cid; + this.payload = payload; + } async decrypt(webhookId) { const jsonPayload = await EncryptedPayload.crypto.decrypt(this.payload).catch((err) => { - winston.error("Failed to decrypt state_json", { webhookId: webhookId }); + winston.error("Failed to decrypt state_json", { webhookId }); throw err; }); const tokenPayload = JSON.parse(jsonPayload); @@ -44,5 +38,3 @@ class EncryptedPayload { } } exports.EncryptedPayload = EncryptedPayload; -EncryptedPayload._crypto = undefined; -EncryptedPayload._currentCipherId = undefined; diff --git a/lib/hub/oauth_action.d.ts b/lib/hub/oauth_action.d.ts index 90be31be5..54f45fe78 100644 --- a/lib/hub/oauth_action.d.ts +++ b/lib/hub/oauth_action.d.ts @@ -1,3 +1,4 @@ +import { EncryptedPayload } from "."; import { Action, RouteBuilder } from "./action"; import { ActionRequest } from "./action_request"; export declare abstract class OAuthAction extends Action { @@ -7,5 +8,8 @@ export declare abstract class OAuthAction extends Action { [key: string]: string; }, redirectUri: string): Promise; asJson(router: RouteBuilder, request: ActionRequest): any; + oauthExtractTokensFromStateJson(stateJson: string, requestWebhookId: string | undefined): Promise; + oauthMaybeEncryptTokens(tokenPayload: any, requestWebhookId: string | undefined): Promise; + oauthDecryptTokens(encryptedPayload: EncryptedPayload, requestWebhookId: string | undefined): Promise; } export declare function isOauthAction(action: Action): boolean; diff --git a/lib/hub/oauth_action.js b/lib/hub/oauth_action.js index 794b5531c..4a64720dc 100644 --- a/lib/hub/oauth_action.js +++ b/lib/hub/oauth_action.js @@ -2,6 +2,8 @@ Object.defineProperty(exports, "__esModule", { value: true }); exports.OAuthAction = void 0; exports.isOauthAction = isOauthAction; +const winston = require("winston"); +const _1 = require("."); const action_1 = require("./action"); class OAuthAction extends action_1.Action { asJson(router, request) { @@ -9,6 +11,60 @@ class OAuthAction extends action_1.Action { json.uses_oauth = true; return json; } + async oauthExtractTokensFromStateJson(stateJson, requestWebhookId) { + let state; + try { + state = JSON.parse(stateJson); + } + catch (e) { + winston.error(`Failed to parse state_json`, { webhookId: requestWebhookId, action: this.name }); + return null; + } + if (state.cid && state.payload) { + winston.info("Extracting encrypted state_json", { webhookId: requestWebhookId, action: this.name }); + const encryptedPayload = new _1.EncryptedPayload(state.cid, state.payload); + try { + const tokenPayload = await this.oauthDecryptTokens(encryptedPayload, requestWebhookId); + return tokenPayload; + } + catch (e) { + winston.error(`Failed to decrypt or parse encrypted payload: ${e.message}`, { webhookId: requestWebhookId, action: this.name }); + return null; + } + } + else { + winston.info("Extracting unencrypted state_json", { webhookId: requestWebhookId, action: this.name }); + return state; + } + } + async oauthMaybeEncryptTokens(tokenPayload, requestWebhookId) { + // Generate the per-action environment variable name + // e.g. "salesforce_campaigns" -> "ENCRYPT_PAYLOAD_SALESFORCE_CAMPAIGNS" + const envVarName = `ENCRYPT_PAYLOAD_${this.name.toUpperCase()}`; + const perActionEncryptionValue = process.env[envVarName]; + // Check per-action variable. Default to false if not set. + // We explicitly do NOT fallback to ENCRYPT_PAYLOAD as that is reserved for Google Drive. + const shouldEncrypt = perActionEncryptionValue === "true"; + if (shouldEncrypt) { + const encrypted = await new _1.ActionCrypto().encrypt(JSON.stringify(tokenPayload)).catch((err) => { + winston.error("Encryption not correctly configured", { webhookId: requestWebhookId, action: this.name }); + throw err; + }); + const payload = new _1.EncryptedPayload(_1.EncryptedPayload.currentCipherId, encrypted); + return payload; + } + else { + return JSON.stringify(tokenPayload); + } + } + async oauthDecryptTokens(encryptedPayload, requestWebhookId) { + const actionCrypto = new _1.ActionCrypto(); + const jsonPayload = await actionCrypto.decrypt(encryptedPayload.payload).catch((err) => { + winston.error("Failed to decrypt state_json", { webhookId: requestWebhookId, action: this.name }); + throw err; + }); + return JSON.parse(jsonPayload); + } } exports.OAuthAction = OAuthAction; function isOauthAction(action) { diff --git a/src/actions/airtable/airtable.ts b/src/actions/airtable/airtable.ts index a22caf49a..763e853b5 100644 --- a/src/actions/airtable/airtable.ts +++ b/src/actions/airtable/airtable.ts @@ -54,31 +54,33 @@ export class AirtableAction extends Hub.OAuthAction { try { let accessToken if (request.params.state_json) { - const stateJson = JSON.parse(request.params.state_json) + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId) accessToken = stateJson.tokens.access_token - state.data = JSON.stringify({ + const encrypted = await this.oauthMaybeEncryptTokens({ tokens: { refresh_token: stateJson.tokens.refresh_token, access_token: accessToken, }, - }) + }, request.webhookId) + state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) } try { await this.executeAirtable(request, records, accessToken) } catch { if (request.params.state_json) { - const stateJson = JSON.parse(request.params.state_json) + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId) const refreshResponse = await this.refreshTokens(stateJson.tokens.refresh_token) // @ts-ignore if (Object.keys(refreshResponse.data as any).length !== 0) { accessToken = (refreshResponse as any).data.access_token - state.data = JSON.stringify({ + const encrypted = await this.oauthMaybeEncryptTokens({ tokens: { refresh_token: (refreshResponse as any).data.refresh_token, access_token: accessToken, }, - }) + }, request.webhookId) + state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) } else { delete state.data } @@ -110,29 +112,36 @@ export class AirtableAction extends Hub.OAuthAction { const form = new Hub.ActionForm() try { let accessToken + let stateJson if (request.params.state_json) { - const stateJson = JSON.parse(request.params.state_json) + stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId) accessToken = stateJson.tokens.access_token } try { await this.checkBaseList(accessToken) if (form.state === undefined) { form.state = new ActionState() - form.state.data = request.params.state_json + if (stateJson) { + const encrypted = await this.oauthMaybeEncryptTokens(stateJson, request.webhookId) + form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) + } } } catch { // Assume the failure is due to Oauth failure, // refresh token and retry once. if (request.params.state_json) { - const stateJson = JSON.parse(request.params.state_json) + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId) const refreshResponse = await this.refreshTokens(stateJson.tokens.refresh_token) if (Object.keys(refreshResponse.data as any).length !== 0) { accessToken = (refreshResponse as any).data.access_token form.state = new ActionState() - form.state.data = JSON.stringify({tokens: { + const encrypted = await this.oauthMaybeEncryptTokens({ + tokens: { refresh_token: (refreshResponse as any).data.refresh_token, access_token: accessToken, - }}) + } + }, request.webhookId) + form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) } } await this.checkBaseList(accessToken) diff --git a/src/actions/dropbox/dropbox.ts b/src/actions/dropbox/dropbox.ts index bee01a100..1856071db 100644 --- a/src/actions/dropbox/dropbox.ts +++ b/src/actions/dropbox/dropbox.ts @@ -17,19 +17,19 @@ export class DropboxAction extends Hub.OAuthAction { requiredFields = [] params = [] - async execute(request: Hub.ActionRequest) { + async execute(request: Hub.ActionRequest): Promise { const filename = this.dropboxFilename(request) const directory = request.formParams.directory const ext = request.attachment!.fileExtension let accessToken = "" if (request.params.state_json) { - const stateJson = JSON.parse(request.params.state_json) + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId) if (stateJson.code && stateJson.redirect) { accessToken = await this.getAccessTokenFromCode(stateJson) } } - const drop = this.dropboxClientFromRequest(request, accessToken) + const drop = await this.dropboxClientFromRequest(request, accessToken) const resp = new Hub.ActionResponse() resp.success = true @@ -56,13 +56,13 @@ export class DropboxAction extends Hub.OAuthAction { let accessToken = "" if (request.params.state_json) { try { - const stateJson = JSON.parse(request.params.state_json) + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId) if (stateJson.code && stateJson.redirect) { accessToken = await this.getAccessTokenFromCode(stateJson) } } catch { winston.warn("Could not parse state_json") } } - const drop = this.dropboxClientFromRequest(request, accessToken) + const drop = await this.dropboxClientFromRequest(request, accessToken) try { const response = await drop.filesListFolder({path: ""}) const folderList = response.entries.filter((entries) => (entries[".tag"] === "folder")) @@ -97,9 +97,11 @@ export class DropboxAction extends Hub.OAuthAction { }], }] if (accessToken !== "") { - const newState = JSON.stringify({access_token: accessToken}) + if (accessToken !== "") { + const encrypted = await this.oauthMaybeEncryptTokens({ access_token: accessToken }, request.webhookId) form.state = new Hub.ActionState() - form.state.data = newState + form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) + } } return form } catch (_error) { @@ -149,7 +151,7 @@ export class DropboxAction extends Hub.OAuthAction { } async oauthCheck(request: Hub.ActionRequest) { - const drop = this.dropboxClientFromRequest(request, "") + const drop = await this.dropboxClientFromRequest(request, "") try { await drop.filesListFolder({path: ""}) return true @@ -186,10 +188,10 @@ export class DropboxAction extends Hub.OAuthAction { return response.access_token } - protected dropboxClientFromRequest(request: Hub.ActionRequest, token: string) { + protected async dropboxClientFromRequest(request: Hub.ActionRequest, token: string) { if (request.params.state_json && token === "") { try { - const json = JSON.parse(request.params.state_json) + const json = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId) token = json.access_token } catch (er) { winston.error("cannot parse") diff --git a/src/actions/facebook/facebook_custom_audiences.ts b/src/actions/facebook/facebook_custom_audiences.ts index b0156cf55..805eaac4a 100644 --- a/src/actions/facebook/facebook_custom_audiences.ts +++ b/src/actions/facebook/facebook_custom_audiences.ts @@ -112,10 +112,11 @@ export class FacebookCustomAudiencesAction extends Hub.OAuthAction { // So now we use that state url to persist the oauth tokens try { + const encrypted = await this.oauthMaybeEncryptTokens(userState, payload.webhookId) await gaxios.request({ method: "POST", url: payload.stateUrl, - data: userState, + data: typeof encrypted === "string" ? encrypted : encrypted, }) } catch (err: any) { sanitizeError(err) @@ -174,8 +175,8 @@ export class FacebookCustomAudiencesAction extends Hub.OAuthAction { protected async getAccessTokenFromRequest(request: Hub.ActionRequest): Promise { try { - const params: any = request.params - return JSON.parse(params.state_json).tokens.longLivedToken + const stateJson = await this.oauthExtractTokensFromStateJson(`${request.params.state_json}`, request.webhookId!) + return stateJson.tokens.longLivedToken } catch (e: any) { winston.error(`${LOG_PREFIX} Failed to parse state for access token.`, {webhookId: request.webhookId}) return null diff --git a/src/actions/facebook/test_facebook_custom_audiences.ts b/src/actions/facebook/test_facebook_custom_audiences.ts index 346a6be70..4eb606ecd 100644 --- a/src/actions/facebook/test_facebook_custom_audiences.ts +++ b/src/actions/facebook/test_facebook_custom_audiences.ts @@ -169,7 +169,7 @@ describe(`${action.constructor.name} class`, () => { const expectedPostArgs = { method: "POST", url: stateUrl, - data: {tokens: stubTokens, redirect: redirectUri}, + data: JSON.stringify({ tokens: stubTokens, redirect: redirectUri }), } await action.oauthFetchInfo({code: oauthCode, state: encryptedPayload}, redirectUri) diff --git a/src/actions/google/ads/customer_match.ts b/src/actions/google/ads/customer_match.ts index c1bdcce08..7d0115f30 100644 --- a/src/actions/google/ads/customer_match.ts +++ b/src/actions/google/ads/customer_match.ts @@ -82,7 +82,8 @@ export class GoogleAdsCustomerMatch const adsRequest = await GoogleAdsActionRequest.fromHub(hubReq, this, log) await adsRequest.execute() log("info", "Execution complete") - return wrappedResp.returnSuccess(adsRequest.userState) + const encrypted = await this.oauthMaybeEncryptTokens(adsRequest.userState, hubReq.webhookId) + return wrappedResp.returnSuccess(encrypted) } catch (err: any) { sanitizeError(err) makeBetterErrorMessage(err, hubReq.webhookId) @@ -98,7 +99,8 @@ export class GoogleAdsCustomerMatch try { const adsWorker = await GoogleAdsActionRequest.fromHub(hubReq, this, log) wrappedResp.form = await adsWorker.makeForm() - return wrappedResp.returnSuccess(adsWorker.userState) + const encrypted = await this.oauthMaybeEncryptTokens(adsWorker.userState, hubReq.webhookId) + return wrappedResp.returnSuccess(encrypted) // Use this code if you need to force a state reset and redo oauth login // wrappedResp.form = await this.oauthHelper.makeLoginForm(hubReq) // wrappedResp.resetState() diff --git a/src/actions/google/ads/lib/ads_request.ts b/src/actions/google/ads/lib/ads_request.ts index 0778e411f..f55060d3f 100644 --- a/src/actions/google/ads/lib/ads_request.ts +++ b/src/actions/google/ads/lib/ads_request.ts @@ -4,7 +4,7 @@ import * as Hub from "../../../../hub" import { Logger } from "../../common/logger" import { MissingAuthError } from "../../common/missing_auth_error" import { MissingRequiredParamsError } from "../../common/missing_required_params_error" -import { safeParseJson } from "../../common/utils" + import { GoogleAdsCustomerMatch } from "../customer_match" import { GoogleAdsActionExecutor} from "./ads_executor" import { GoogleAdsActionFormBuilder } from "./ads_form_builder" @@ -20,7 +20,11 @@ const LOG_PREFIX = "[G Ads Customer Match]" export class GoogleAdsActionRequest { static async fromHub(hubRequest: Hub.ActionRequest, action: GoogleAdsCustomerMatch, logger: Logger) { - const adsReq = new GoogleAdsActionRequest(hubRequest, action, logger) + const userState = await action.oauthExtractTokensFromStateJson( + `${hubRequest.params.state_json}`, + hubRequest.webhookId!, + ) + const adsReq = new GoogleAdsActionRequest(hubRequest, action, logger, userState) await adsReq.checkTokens() adsReq.setApiClient() return adsReq @@ -36,9 +40,10 @@ export class GoogleAdsActionRequest { readonly hubRequest: Hub.ActionRequest, readonly actionInstance: GoogleAdsCustomerMatch, readonly log: Logger, + userState: any, ) { - const state = safeParseJson(`${hubRequest.params.state_json}`) + const state = userState if (!state || !state.tokens || !state.tokens.access_token || !state.tokens.refresh_token || !state.redirect) { winston.warn( diff --git a/src/actions/google/ads/test_customer_match.ts b/src/actions/google/ads/test_customer_match.ts index a33be16e1..5adc3fecf 100644 --- a/src/actions/google/ads/test_customer_match.ts +++ b/src/actions/google/ads/test_customer_match.ts @@ -190,7 +190,7 @@ describe(`${action.constructor.name} class`, () => { const expectedPostArgs = { method: "POST", url: stateUrl, - data: {tokens: stubTokens, redirect: redirectUri}, + data: JSON.stringify({ tokens: stubTokens, redirect: redirectUri }), } await action.oauthFetchInfo({code: oauthCode, state: encryptedPayload}, redirectUri) diff --git a/src/actions/google/analytics/data_import.ts b/src/actions/google/analytics/data_import.ts index 7c151203c..78a6d2784 100644 --- a/src/actions/google/analytics/data_import.ts +++ b/src/actions/google/analytics/data_import.ts @@ -104,7 +104,8 @@ export class GoogleAnalyticsDataImportAction // Since the upload was successful, update the lastUsedFormParams in user state gaWorker.setLastUsedFormParams() - wrappedResp.setUserState(gaWorker.userState) + const encrypted = await this.oauthMaybeEncryptTokens(gaWorker.userState, hubReq.webhookId) + wrappedResp.setUserState(encrypted) if (gaWorker.isDeleteOtherFiles) { currentStep = "Delete other files step" diff --git a/src/actions/google/analytics/lib/ga_worker.ts b/src/actions/google/analytics/lib/ga_worker.ts index 95cd8095d..7275dbadd 100644 --- a/src/actions/google/analytics/lib/ga_worker.ts +++ b/src/actions/google/analytics/lib/ga_worker.ts @@ -4,7 +4,7 @@ import * as Hub from "../../../../hub" import { Logger } from "../../common/logger" import { MissingAuthError } from "../../common/missing_auth_error" import { MissingRequiredParamsError } from "../../common/missing_required_params_error" -import { safeParseJson } from "../../common/utils" + import { GoogleAnalyticsDataImportAction } from "../data_import" import { CsvHeaderTransformStream } from "./csv_header_transform_stream" @@ -21,7 +21,11 @@ export class GoogleAnalyticsActionWorker { actionInstance: GoogleAnalyticsDataImportAction, logger: Logger, ) { - const gaWorker = new GoogleAnalyticsActionWorker(hubRequest, actionInstance, logger) + const userState = await actionInstance.oauthExtractTokensFromStateJson( + `${hubRequest.params.state_json}`, + hubRequest.webhookId!, + ) + const gaWorker = new GoogleAnalyticsActionWorker(hubRequest, actionInstance, logger, userState) return gaWorker } @@ -34,8 +38,9 @@ export class GoogleAnalyticsActionWorker { readonly hubRequest: Hub.ActionRequest, readonly actionInstance: GoogleAnalyticsDataImportAction, readonly log: Logger, + userState: any, ) { - const tmpState = safeParseJson(hubRequest.params.state_json) + const tmpState = userState if (!tmpState || !tmpState.tokens || !tmpState.redirect) { throw new MissingAuthError("User state was missing or did not contain tokens & redirect") diff --git a/src/actions/google/analytics/test_data_import.ts b/src/actions/google/analytics/test_data_import.ts index 294d1553f..c3c81d091 100644 --- a/src/actions/google/analytics/test_data_import.ts +++ b/src/actions/google/analytics/test_data_import.ts @@ -727,7 +727,7 @@ describe(`${action.constructor.name} class`, () => { const expectedPostArgs = { method: "POST", url: stateUrl, - data: {tokens: stubTokens, redirect: redirectUri}, + data: JSON.stringify({ tokens: stubTokens, redirect: redirectUri }), } await action.oauthFetchInfo({code: oauthCode, state: encryptedPayload}, redirectUri) diff --git a/src/actions/google/common/oauth_helper.ts b/src/actions/google/common/oauth_helper.ts index 59f1581ff..8ad43d433 100644 --- a/src/actions/google/common/oauth_helper.ts +++ b/src/actions/google/common/oauth_helper.ts @@ -119,10 +119,11 @@ export class GoogleOAuthHelper { // So now we use that state url to persist the oauth tokens try { + const encrypted = await this.actionInstance.oauthMaybeEncryptTokens(userState, webhookId) await gaxios.request({ method: "POST", url: payload.stateUrl, - data: userState, + data: typeof encrypted === "string" ? encrypted : encrypted, }) } catch (err: any) { // We have seen weird behavior where Looker correctly updates the state, but returns a nonsense status code diff --git a/src/actions/google/common/wrapped_response.ts b/src/actions/google/common/wrapped_response.ts index 5790e4bcc..4ac1318bb 100644 --- a/src/actions/google/common/wrapped_response.ts +++ b/src/actions/google/common/wrapped_response.ts @@ -47,6 +47,6 @@ export class WrappedResponse { setUserState(userState: any) { this._hubResp.state = new Hub.ActionState() - this._hubResp.state.data = JSON.stringify(userState) + this._hubResp.state.data = typeof userState === "string" ? userState : JSON.stringify(userState) } } diff --git a/src/actions/salesforce/campaigns/salesforce_campaigns.ts b/src/actions/salesforce/campaigns/salesforce_campaigns.ts index 1fcf8349a..a9832715a 100644 --- a/src/actions/salesforce/campaigns/salesforce_campaigns.ts +++ b/src/actions/salesforce/campaigns/salesforce_campaigns.ts @@ -160,7 +160,7 @@ export class SalesforceCampaignsAction extends Hub.OAuthAction { let tokens: Tokens try { - const stateJson = JSON.parse(request.params.state_json) + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId) if (stateJson.access_token && stateJson.refresh_token) { tokens = stateJson } else { @@ -179,7 +179,8 @@ export class SalesforceCampaignsAction extends Hub.OAuthAction { response.message = message tokens = { access_token: sfdcConn.accessToken, refresh_token: sfdcConn.refreshToken } response.state = new Hub.ActionState() - response.state.data = JSON.stringify(tokens) + const encrypted = await this.oauthMaybeEncryptTokens(tokens, request.webhookId) + response.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) } catch (e: any) { response = { success: false, message: e.message } } @@ -205,13 +206,14 @@ export class SalesforceCampaignsAction extends Hub.OAuthAction { // scenarios 1 and 2 will show loginForm, 3 and 4 will show formBuilder if (request.params.state_json) { try { - const stateJson = JSON.parse(request.params.state_json) + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId) if (stateJson.access_token && stateJson.refresh_token) { tokens = stateJson } else { tokens = await this.sfdcOauthHelper.getAccessTokensFromAuthCode(stateJson) form.state = new Hub.ActionState() - form.state.data = JSON.stringify(tokens) + const encrypted = await this.oauthMaybeEncryptTokens(tokens, request.webhookId) + form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) } // passing back connection object to handle access token refresh and update state @@ -220,7 +222,8 @@ export class SalesforceCampaignsAction extends Hub.OAuthAction { form.fields = fields tokens = { access_token: sfdcConn.accessToken, refresh_token: sfdcConn.refreshToken } form.state = new Hub.ActionState() - form.state.data = JSON.stringify(tokens) + const encrypted = await this.oauthMaybeEncryptTokens(tokens, request.webhookId) + form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) return form } catch (e: any) { diff --git a/src/hub/encrypted_payload.ts b/src/hub/encrypted_payload.ts index 69ed196af..979864b61 100644 --- a/src/hub/encrypted_payload.ts +++ b/src/hub/encrypted_payload.ts @@ -6,17 +6,11 @@ import * as winston from "winston" export class EncryptedPayload { static get crypto() { - if (this._crypto === undefined) { - this._crypto = new ActionCrypto() - } - return this._crypto + return new ActionCrypto() } static get currentCipherId() { - if (this._currentCipherId === undefined) { - this._currentCipherId = this.crypto.cipherId() - } - return this._currentCipherId + return this.crypto.cipherId() } static async encrypt( @@ -30,8 +24,7 @@ export class EncryptedPayload { }) return new EncryptedPayload(this.currentCipherId, encrypted) } - private static _crypto: ActionCrypto | undefined = undefined - private static _currentCipherId: string | undefined = undefined + constructor(public cid: string, public payload: string) {} diff --git a/src/hub/oauth_action.ts b/src/hub/oauth_action.ts index 505ca4f1b..077dadf2f 100644 --- a/src/hub/oauth_action.ts +++ b/src/hub/oauth_action.ts @@ -1,3 +1,5 @@ +import * as winston from "winston" +import { ActionCrypto, EncryptedPayload } from "." import {Action, RouteBuilder} from "./action" import {ActionRequest} from "./action_request" @@ -11,6 +13,83 @@ export abstract class OAuthAction extends Action { json.uses_oauth = true return json } + + async oauthExtractTokensFromStateJson( + stateJson: string, + requestWebhookId: string | undefined, + ): Promise { + let state: any + try { + state = JSON.parse(stateJson) + } catch (e: any) { + winston.error( + `Failed to parse state_json`, + { webhookId: requestWebhookId, action: this.name }, + ) + return null + } + + if (state.cid && state.payload) { + winston.info("Extracting encrypted state_json", { webhookId: requestWebhookId, action: this.name }) + const encryptedPayload = new EncryptedPayload(state.cid, state.payload) + try { + const tokenPayload = await this.oauthDecryptTokens( + encryptedPayload, + requestWebhookId, + ) + return tokenPayload + } catch (e: any) { + winston.error( + `Failed to decrypt or parse encrypted payload: ${e.message}`, + { webhookId: requestWebhookId, action: this.name }, + ) + return null + } + } else { + winston.info("Extracting unencrypted state_json", { webhookId: requestWebhookId, action: this.name }) + return state + } + } + + async oauthMaybeEncryptTokens( + tokenPayload: any, + requestWebhookId: string | undefined, + ): Promise { + // Generate the per-action environment variable name + // e.g. "salesforce_campaigns" -> "ENCRYPT_PAYLOAD_SALESFORCE_CAMPAIGNS" + const envVarName = `ENCRYPT_PAYLOAD_${this.name.toUpperCase()}` + const perActionEncryptionValue = process.env[envVarName] + + // Check per-action variable. Default to false if not set. + // We explicitly do NOT fallback to ENCRYPT_PAYLOAD as that is reserved for Google Drive. + const shouldEncrypt = perActionEncryptionValue === "true" + + if (shouldEncrypt) { + const encrypted = await new ActionCrypto().encrypt(JSON.stringify(tokenPayload)).catch((err: string) => { + winston.error("Encryption not correctly configured", { webhookId: requestWebhookId, action: this.name }) + throw err + }) + const payload = new EncryptedPayload( + EncryptedPayload.currentCipherId, + encrypted, + ) + return payload + } else { + return JSON.stringify(tokenPayload) + } + } + + async oauthDecryptTokens( + encryptedPayload: EncryptedPayload, + requestWebhookId: string | undefined, + ): Promise { + const actionCrypto = new ActionCrypto() + const jsonPayload = await actionCrypto.decrypt(encryptedPayload.payload).catch((err: string) => { + winston.error("Failed to decrypt state_json", { webhookId: requestWebhookId, action: this.name }) + throw err + }) + return JSON.parse(jsonPayload) + } } export function isOauthAction(action: Action): boolean { diff --git a/test/test.ts b/test/test.ts index ced77333d..5c5b16eb4 100644 --- a/test/test.ts +++ b/test/test.ts @@ -18,6 +18,7 @@ import "../src/error_types/test_error_utils" import "./test_action_request" import "./test_action_response" import "./test_actions" +import "./test_oauth_action" import "./test_json_detail_stream" // import "./test_server" import "./test_smoke" diff --git a/test/test_oauth_action.ts b/test/test_oauth_action.ts new file mode 100644 index 000000000..0faf9a264 --- /dev/null +++ b/test/test_oauth_action.ts @@ -0,0 +1,107 @@ +import * as chai from "chai" +import * as sinon from "sinon" + +import * as Hub from "../src/hub" +import { ActionCrypto } from "../src/hub" + +class TestOAuthAction extends Hub.OAuthAction { + name = "test_oauth" + label = "Test OAuth" + description = "Test OAuth Action" + supportedActionTypes = [Hub.ActionType.Dashboard] + params = [] + + async execute(_request: Hub.ActionRequest) { + return new Hub.ActionResponse() + } + + async form(_request: Hub.ActionRequest) { + return new Hub.ActionForm() + } + + async oauthCheck(_request: Hub.ActionRequest) { + return true + } + + async oauthUrl(_redirectUri: string, _encryptedState: string) { + return "" + } + + async oauthFetchInfo(_urlParams: { [key: string]: string }, _redirectUri: string) { + // pass + } +} + +describe("OAuthAction Encryption", () => { + let action: TestOAuthAction + let encryptStub: sinon.SinonStub + let decryptStub: sinon.SinonStub + + beforeEach(() => { + action = new TestOAuthAction() + encryptStub = sinon.stub(ActionCrypto.prototype, "encrypt").resolves("encrypted_data") + decryptStub = sinon.stub(ActionCrypto.prototype, "decrypt").resolves(JSON.stringify({ tokens: "secret" })) + process.env.CIPHER_MASTER = "secret" + process.env.CIPHER_PASSWORD = "password" + }) + + afterEach(() => { + encryptStub.restore() + decryptStub.restore() + process.env.ENCRYPT_PAYLOAD = "false" + delete process.env.CIPHER_MASTER + delete process.env.CIPHER_PASSWORD + }) + + describe("oauthMaybeEncryptTokens", () => { + it("returns plain JSON when actions specific env var is not set (defaults to false)", async () => { + // Even if global is true, we should default to false for OAuthAction if specific var is missing + process.env.ENCRYPT_PAYLOAD = "true" + const payload = { tokens: "secret" } + const result = await action.oauthMaybeEncryptTokens(payload, "webhookId") + chai.expect(result).to.equal(JSON.stringify(payload)) + sinon.assert.notCalled(encryptStub) + }) + + it("returns EncryptedPayload when action specific env var is true", async () => { + process.env.ENCRYPT_PAYLOAD_TEST_OAUTH = "true" + const payload = { tokens: "secret" } + const result = await action.oauthMaybeEncryptTokens(payload, "webhookId") + chai.expect(result).to.be.instanceOf(Hub.EncryptedPayload) + if (result instanceof Hub.EncryptedPayload) { + chai.expect(result.payload).to.equal("encrypted_data") + } + sinon.assert.calledOnce(encryptStub) + delete process.env.ENCRYPT_PAYLOAD_TEST_OAUTH + }) + + + }) + + describe("oauthExtractTokensFromStateJson", () => { + it("extracts unencrypted tokens", async () => { + const state = { tokens: "secret" } + const result = await action.oauthExtractTokensFromStateJson(JSON.stringify(state), "webhookId") + chai.expect(result).to.deep.equal(state) + }) + + it("decrypts encrypted tokens", async () => { + const state = { cid: "cid", payload: "encrypted_payload" } + const result = await action.oauthExtractTokensFromStateJson(JSON.stringify(state), "webhookId") + chai.expect(result).to.deep.equal({ tokens: "secret" }) + sinon.assert.calledWith(decryptStub, "encrypted_payload") + }) + + it("returns null on invalid JSON", async () => { + const result = await action.oauthExtractTokensFromStateJson("invalid_json", "webhookId") + chai.expect(result).to.be.null + }) + + it("returns null on decryption failure", async () => { + decryptStub.rejects(new Error("Decryption failed")) + const state = { cid: "cid", payload: "encrypted_payload" } + const result = await action.oauthExtractTokensFromStateJson(JSON.stringify(state), "webhookId") + chai.expect(result).to.be.null + }) + }) +}) From ba19eca2c2e8c0e028f5aa75c19295f90ca75c8c Mon Sep 17 00:00:00 2001 From: First Name Date: Thu, 26 Feb 2026 21:20:31 +0000 Subject: [PATCH 3/4] Fix Airtable OAuth state vulnerability - Implemented oauthMaybeEncryptTokens and oauthExtractTokensFromStateJson in form() and execute() - Fixed double-stringification bugs during token serialization - Updated AirtableTokens to preserve redirectUri across refresh loops - Added backward compatibility in oauthCheck() for legacy, unencrypted state_json formats - Ensured 100% test coverage and addressed all linter warnings --- src/actions/airtable/airtable.ts | 129 ++++++++++-------- src/actions/airtable/airtable_tokens.ts | 38 ++++++ src/actions/airtable/test_airtable.ts | 15 ++ src/actions/dropbox/dropbox.ts | 6 +- .../campaigns/salesforce_campaigns.ts | 4 +- src/hub/encrypted_payload.ts | 1 - test/test.ts | 2 +- test/test_oauth_action.ts | 2 - 8 files changed, 127 insertions(+), 70 deletions(-) create mode 100644 src/actions/airtable/airtable_tokens.ts diff --git a/src/actions/airtable/airtable.ts b/src/actions/airtable/airtable.ts index 763e853b5..be66c2024 100644 --- a/src/actions/airtable/airtable.ts +++ b/src/actions/airtable/airtable.ts @@ -4,7 +4,8 @@ import * as crypto from "crypto" import * as gaxios from "gaxios" import * as qs from "qs" import * as winston from "winston" -import {ActionResponse, ActionState} from "../../hub" +import { ActionResponse, ActionState } from "../../hub" +import { AirtableTokens } from "./airtable_tokens" const airtable: any = require("airtable") @@ -49,44 +50,44 @@ export class AirtableAction extends Hub.OAuthAction { return record }) - const response = new ActionResponse({success: true}) + const response = new ActionResponse({ success: true }) const state = new ActionState() try { - let accessToken + let accessToken: string | undefined + let tokens: AirtableTokens | undefined if (request.params.state_json) { - const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId) - accessToken = stateJson.tokens.access_token - const encrypted = await this.oauthMaybeEncryptTokens({ - tokens: { - refresh_token: stateJson.tokens.refresh_token, - access_token: accessToken, - }, - }, request.webhookId) + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId!) + tokens = AirtableTokens.fromJson(stateJson) + accessToken = tokens.access_token + const encrypted = await this.oauthMaybeEncryptTokens(new AirtableTokens( + tokens.refresh_token, + accessToken, + tokens.redirectUri, + ), request.webhookId!) state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) } try { - await this.executeAirtable(request, records, accessToken) + await this.executeAirtable(request, records, accessToken as string) } catch { - if (request.params.state_json) { - const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId) + if (request.params.state_json && tokens) { + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId!) const refreshResponse = await this.refreshTokens(stateJson.tokens.refresh_token) - // @ts-ignore - if (Object.keys(refreshResponse.data as any).length !== 0) { - accessToken = (refreshResponse as any).data.access_token - const encrypted = await this.oauthMaybeEncryptTokens({ - tokens: { - refresh_token: (refreshResponse as any).data.refresh_token, - access_token: accessToken, - }, - }, request.webhookId) + const refreshData = refreshResponse.data as Record + if (Object.keys(refreshData).length !== 0) { + accessToken = refreshData.access_token + const encrypted = await this.oauthMaybeEncryptTokens(new AirtableTokens( + refreshData.refresh_token, + accessToken, + tokens.redirectUri, + ), request.webhookId!) state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) } else { delete state.data } } // Try again one more time - await this.executeAirtable(request, records, accessToken) + await this.executeAirtable(request, records, accessToken as string) } } catch (e: any) { response.success = false @@ -111,40 +112,43 @@ export class AirtableAction extends Hub.OAuthAction { async form(request: Hub.ActionRequest) { const form = new Hub.ActionForm() try { - let accessToken - let stateJson + let accessToken: string | undefined + let tokens: AirtableTokens | undefined if (request.params.state_json) { - stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId) - accessToken = stateJson.tokens.access_token + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId!) + tokens = AirtableTokens.fromJson(stateJson) + accessToken = tokens.access_token } try { - await this.checkBaseList(accessToken) + await this.checkBaseList(accessToken as string) if (form.state === undefined) { form.state = new ActionState() - if (stateJson) { - const encrypted = await this.oauthMaybeEncryptTokens(stateJson, request.webhookId) - form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) + if (tokens) { + const encrypted = await this.oauthMaybeEncryptTokens(tokens, request.webhookId!) + const encryptedStr = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) + request.params.state_json = encryptedStr + form.state.data = encryptedStr } } } catch { // Assume the failure is due to Oauth failure, // refresh token and retry once. - if (request.params.state_json) { - const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId) + if (request.params.state_json && tokens) { + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId!) const refreshResponse = await this.refreshTokens(stateJson.tokens.refresh_token) - if (Object.keys(refreshResponse.data as any).length !== 0) { - accessToken = (refreshResponse as any).data.access_token + const refreshData = refreshResponse.data as Record + if (Object.keys(refreshData).length !== 0) { + accessToken = refreshData.access_token form.state = new ActionState() - const encrypted = await this.oauthMaybeEncryptTokens({ - tokens: { - refresh_token: (refreshResponse as any).data.refresh_token, - access_token: accessToken, - } - }, request.webhookId) + const encrypted = await this.oauthMaybeEncryptTokens(new AirtableTokens( + refreshData.refresh_token, + accessToken, + tokens.redirectUri, + ), request.webhookId!) form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) } } - await this.checkBaseList(accessToken) + await this.checkBaseList(accessToken as string) } form.fields = [{ label: "Airtable Base", @@ -162,7 +166,7 @@ export class AirtableAction extends Hub.OAuthAction { const codeVerifier = crypto.randomBytes(96).toString("base64url") // 128 characters const actionCrypto = new Hub.ActionCrypto() - const jsonString = JSON.stringify({stateurl: request.params.state_url, verifier: codeVerifier}) + const jsonString = JSON.stringify({ stateurl: request.params.state_url, verifier: codeVerifier }) const ciphertextBlob = await actionCrypto.encrypt(jsonString).catch((err: string) => { winston.error("Encryption not correctly configured") throw err @@ -178,7 +182,13 @@ export class AirtableAction extends Hub.OAuthAction { return form } - async oauthCheck(_request: Hub.ActionRequest) { + async oauthCheck(request: Hub.ActionRequest) { + if (request.params.state_json) { + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId!) + if (stateJson) { + return true + } + } return false } @@ -198,7 +208,7 @@ export class AirtableAction extends Hub.OAuthAction { code: urlParams.code, }) const encodedCreds = Buffer.from(`${process.env.AIRTABLE_CLIENT_ID}:${process.env.AIRTABLE_CLIENT_SECRET}`) - .toString("base64") + .toString("base64") const response = await gaxios.request({ method: "POST", url: "https://www.airtable.com/oauth2/v1/token", @@ -211,13 +221,12 @@ export class AirtableAction extends Hub.OAuthAction { // Pass back context to Looker if (response.status === 200) { const data: any = response.data + const tokenPayload = new AirtableTokens(data.refresh_token, data.access_token, redirectUri) + const encrypted = await this.oauthMaybeEncryptTokens(tokenPayload, undefined) await gaxios.request({ url: payload.stateurl, method: "POST", - body: JSON.stringify({tokens: { - refresh_token: data.refresh_token, - access_token: data.access_token, - }, redirect: redirectUri}), + body: encrypted, }).catch((_err) => { winston.error(_err.toString()) }) } else { winston.warn("Oauth for Airtable unsuccessful") @@ -227,7 +236,7 @@ export class AirtableAction extends Hub.OAuthAction { async oauthUrl(redirectUri: string, encryptedState: string) { - const clientId = process.env.AIRTABLE_CLIENT_ID ? process.env.AIRTABLE_CLIENT_ID : "must exist" + const clientId = process.env.AIRTABLE_CLIENT_ID ? process.env.AIRTABLE_CLIENT_ID : "must exist" const actionCrypto = new Hub.ActionCrypto() const plaintext = await actionCrypto.decrypt(encryptedState).catch((err: string) => { @@ -240,12 +249,12 @@ export class AirtableAction extends Hub.OAuthAction { const codeVerifier = payload.verifier// 128 characters const codeChallengeMethod = "S256" const codeChallenge = crypto - .createHash("sha256") - .update(codeVerifier) // hash the code verifier with the sha256 algorithm - .digest("base64") // base64 encode, needs to be transformed to base64url - .replace(/=/g, "") // remove = - .replace(/\+/g, "-") // replace + with - - .replace(/\//g, "_") // replace / with _ now base64url encoded + .createHash("sha256") + .update(codeVerifier) // hash the code verifier with the sha256 algorithm + .digest("base64") // base64 encode, needs to be transformed to base64url + .replace(/=/g, "") // remove = + .replace(/\+/g, "-") // replace + with - + .replace(/\//g, "_") // replace / with _ now base64url encoded // build the authorization URL const authorizationUrl = new URL(`https://www.airtable.com/oauth2/v1/authorize`) authorizationUrl.searchParams.set("code_challenge", codeChallenge) @@ -261,7 +270,7 @@ export class AirtableAction extends Hub.OAuthAction { } private async airtableClientFromRequest(token: string) { - return new airtable({apiKey: token}) + return new airtable({ apiKey: token }) } private async refreshTokens(refreshToken: string) { @@ -273,7 +282,7 @@ export class AirtableAction extends Hub.OAuthAction { }) const encodedCreds = Buffer.from(`${process.env.AIRTABLE_CLIENT_ID}:${process.env.AIRTABLE_CLIENT_SECRET}`) - .toString("base64") + .toString("base64") return await gaxios.request({ method: "POST", url: "https://www.airtable.com/oauth2/v1/token", @@ -289,7 +298,7 @@ export class AirtableAction extends Hub.OAuthAction { errorMessage = errorMessage + ` returning http code ${e.code}` } winston.warn(errorMessage) - return {data: {}} + return { data: {} } } } diff --git a/src/actions/airtable/airtable_tokens.ts b/src/actions/airtable/airtable_tokens.ts new file mode 100644 index 000000000..4aad6aaae --- /dev/null +++ b/src/actions/airtable/airtable_tokens.ts @@ -0,0 +1,38 @@ +import { TokenPayload } from "../../hub" + +export class AirtableTokens extends TokenPayload { + + static fromJson(json: any): AirtableTokens { + if (json.tokens) { + return new AirtableTokens(json.tokens.refresh_token, json.tokens.access_token, json.redirect) + } + return new AirtableTokens(json.refresh_token, json.access_token, json.redirectUri) + } + + // tslint:disable-next-line:variable-name + refresh_token: string + // tslint:disable-next-line:variable-name + access_token: string + redirectUri?: string + + constructor(refreshToken: string, accessToken: string, redirectUri?: string) { + super() + this.refresh_token = refreshToken + this.access_token = accessToken + this.redirectUri = redirectUri + } + + asJson(): any { + return { + tokens: { + refresh_token: this.refresh_token, + access_token: this.access_token, + }, + redirect: this.redirectUri, + } + } + + toJSON(): any { + return this.asJson() + } +} diff --git a/src/actions/airtable/test_airtable.ts b/src/actions/airtable/test_airtable.ts index 194bee1b0..5cee0b7a6 100644 --- a/src/actions/airtable/test_airtable.ts +++ b/src/actions/airtable/test_airtable.ts @@ -244,6 +244,21 @@ describe(`${action.constructor.name} unit tests`, () => { }) + describe("oauthCheck", () => { + it("returns true for valid legacy unencrypted state", async () => { + const request = new Hub.ActionRequest() + request.params.state_json = "{\"tokens\": {\"refresh_token\": \"lol\",\"access_token\":\"test\"}}" + const result = await action.oauthCheck(request) + chai.expect(result).to.equal(true) + }) + + it("returns false for missing state", async () => { + const request = new Hub.ActionRequest() + const result = await action.oauthCheck(request) + chai.expect(result).to.equal(false) + }) + }) + describe("form", () => { it("has form", () => { diff --git a/src/actions/dropbox/dropbox.ts b/src/actions/dropbox/dropbox.ts index 1856071db..fd428629c 100644 --- a/src/actions/dropbox/dropbox.ts +++ b/src/actions/dropbox/dropbox.ts @@ -97,11 +97,9 @@ export class DropboxAction extends Hub.OAuthAction { }], }] if (accessToken !== "") { - if (accessToken !== "") { - const encrypted = await this.oauthMaybeEncryptTokens({ access_token: accessToken }, request.webhookId) + const encrypted = await this.oauthMaybeEncryptTokens({ access_token: accessToken }, request.webhookId) form.state = new Hub.ActionState() - form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) - } + form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) } return form } catch (_error) { diff --git a/src/actions/salesforce/campaigns/salesforce_campaigns.ts b/src/actions/salesforce/campaigns/salesforce_campaigns.ts index a9832715a..3bf9a510e 100644 --- a/src/actions/salesforce/campaigns/salesforce_campaigns.ts +++ b/src/actions/salesforce/campaigns/salesforce_campaigns.ts @@ -222,8 +222,8 @@ export class SalesforceCampaignsAction extends Hub.OAuthAction { form.fields = fields tokens = { access_token: sfdcConn.accessToken, refresh_token: sfdcConn.refreshToken } form.state = new Hub.ActionState() - const encrypted = await this.oauthMaybeEncryptTokens(tokens, request.webhookId) - form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted) + const encryptedState = await this.oauthMaybeEncryptTokens(tokens, request.webhookId) + form.state.data = typeof encryptedState === "string" ? encryptedState : JSON.stringify(encryptedState) return form } catch (e: any) { diff --git a/src/hub/encrypted_payload.ts b/src/hub/encrypted_payload.ts index 979864b61..d26645502 100644 --- a/src/hub/encrypted_payload.ts +++ b/src/hub/encrypted_payload.ts @@ -25,7 +25,6 @@ export class EncryptedPayload { return new EncryptedPayload(this.currentCipherId, encrypted) } - constructor(public cid: string, public payload: string) {} async decrypt(webhookId: string | undefined): Promise { diff --git a/test/test.ts b/test/test.ts index 5c5b16eb4..2b2db4faf 100644 --- a/test/test.ts +++ b/test/test.ts @@ -18,8 +18,8 @@ import "../src/error_types/test_error_utils" import "./test_action_request" import "./test_action_response" import "./test_actions" -import "./test_oauth_action" import "./test_json_detail_stream" +import "./test_oauth_action" // import "./test_server" import "./test_smoke" diff --git a/test/test_oauth_action.ts b/test/test_oauth_action.ts index 0faf9a264..3e09b81ed 100644 --- a/test/test_oauth_action.ts +++ b/test/test_oauth_action.ts @@ -74,8 +74,6 @@ describe("OAuthAction Encryption", () => { sinon.assert.calledOnce(encryptStub) delete process.env.ENCRYPT_PAYLOAD_TEST_OAUTH }) - - }) describe("oauthExtractTokensFromStateJson", () => { From af629903b99d96687c859059584b028a0d2f1499 Mon Sep 17 00:00:00 2001 From: First Name Date: Thu, 26 Feb 2026 21:57:37 +0000 Subject: [PATCH 4/4] Compile typescript --- lib/actions/airtable/airtable.d.ts | 2 +- lib/actions/airtable/airtable.js | 71 +++++++++---------- lib/actions/airtable/airtable_tokens.d.ts | 10 +++ lib/actions/airtable/airtable_tokens.js | 31 ++++++++ lib/actions/dropbox/dropbox.js | 8 +-- .../campaigns/salesforce_campaigns.js | 4 +- 6 files changed, 81 insertions(+), 45 deletions(-) create mode 100644 lib/actions/airtable/airtable_tokens.d.ts create mode 100644 lib/actions/airtable/airtable_tokens.js diff --git a/lib/actions/airtable/airtable.d.ts b/lib/actions/airtable/airtable.d.ts index 935f5c804..95509d1ac 100644 --- a/lib/actions/airtable/airtable.d.ts +++ b/lib/actions/airtable/airtable.d.ts @@ -14,7 +14,7 @@ export declare class AirtableAction extends Hub.OAuthAction { execute(request: Hub.ActionRequest): Promise; checkBaseList(token: string): Promise>; form(request: Hub.ActionRequest): Promise; - oauthCheck(_request: Hub.ActionRequest): Promise; + oauthCheck(request: Hub.ActionRequest): Promise; oauthFetchInfo(urlParams: { [p: string]: string; }, redirectUri: string): Promise; diff --git a/lib/actions/airtable/airtable.js b/lib/actions/airtable/airtable.js index ef2f19ea3..84427ca4a 100644 --- a/lib/actions/airtable/airtable.js +++ b/lib/actions/airtable/airtable.js @@ -7,6 +7,7 @@ const gaxios = require("gaxios"); const qs = require("qs"); const winston = require("winston"); const hub_1 = require("../../hub"); +const airtable_tokens_1 = require("./airtable_tokens"); const airtable = require("airtable"); class AirtableAction extends Hub.OAuthAction { constructor() { @@ -49,33 +50,25 @@ class AirtableAction extends Hub.OAuthAction { const state = new hub_1.ActionState(); try { let accessToken; + let tokens; if (request.params.state_json) { const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); - accessToken = stateJson.tokens.access_token; - const encrypted = await this.oauthMaybeEncryptTokens({ - tokens: { - refresh_token: stateJson.tokens.refresh_token, - access_token: accessToken, - }, - }, request.webhookId); + tokens = airtable_tokens_1.AirtableTokens.fromJson(stateJson); + accessToken = tokens.access_token; + const encrypted = await this.oauthMaybeEncryptTokens(new airtable_tokens_1.AirtableTokens(tokens.refresh_token, accessToken, tokens.redirectUri), request.webhookId); state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); } try { await this.executeAirtable(request, records, accessToken); } catch (_a) { - if (request.params.state_json) { + if (request.params.state_json && tokens) { const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); const refreshResponse = await this.refreshTokens(stateJson.tokens.refresh_token); - // @ts-ignore - if (Object.keys(refreshResponse.data).length !== 0) { - accessToken = refreshResponse.data.access_token; - const encrypted = await this.oauthMaybeEncryptTokens({ - tokens: { - refresh_token: refreshResponse.data.refresh_token, - access_token: accessToken, - }, - }, request.webhookId); + const refreshData = refreshResponse.data; + if (Object.keys(refreshData).length !== 0) { + accessToken = refreshData.access_token; + const encrypted = await this.oauthMaybeEncryptTokens(new airtable_tokens_1.AirtableTokens(refreshData.refresh_token, accessToken, tokens.redirectUri), request.webhookId); state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); } else { @@ -108,36 +101,35 @@ class AirtableAction extends Hub.OAuthAction { const form = new Hub.ActionForm(); try { let accessToken; - let stateJson; + let tokens; if (request.params.state_json) { - stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); - accessToken = stateJson.tokens.access_token; + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); + tokens = airtable_tokens_1.AirtableTokens.fromJson(stateJson); + accessToken = tokens.access_token; } try { await this.checkBaseList(accessToken); if (form.state === undefined) { form.state = new hub_1.ActionState(); - if (stateJson) { - const encrypted = await this.oauthMaybeEncryptTokens(stateJson, request.webhookId); - form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); + if (tokens) { + const encrypted = await this.oauthMaybeEncryptTokens(tokens, request.webhookId); + const encryptedStr = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); + request.params.state_json = encryptedStr; + form.state.data = encryptedStr; } } } catch (_a) { // Assume the failure is due to Oauth failure, // refresh token and retry once. - if (request.params.state_json) { + if (request.params.state_json && tokens) { const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); const refreshResponse = await this.refreshTokens(stateJson.tokens.refresh_token); - if (Object.keys(refreshResponse.data).length !== 0) { - accessToken = refreshResponse.data.access_token; + const refreshData = refreshResponse.data; + if (Object.keys(refreshData).length !== 0) { + accessToken = refreshData.access_token; form.state = new hub_1.ActionState(); - const encrypted = await this.oauthMaybeEncryptTokens({ - tokens: { - refresh_token: refreshResponse.data.refresh_token, - access_token: accessToken, - } - }, request.webhookId); + const encrypted = await this.oauthMaybeEncryptTokens(new airtable_tokens_1.AirtableTokens(refreshData.refresh_token, accessToken, tokens.redirectUri), request.webhookId); form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); } } @@ -173,7 +165,13 @@ class AirtableAction extends Hub.OAuthAction { } return form; } - async oauthCheck(_request) { + async oauthCheck(request) { + if (request.params.state_json) { + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); + if (stateJson) { + return true; + } + } return false; } async oauthFetchInfo(urlParams, redirectUri) { @@ -204,13 +202,12 @@ class AirtableAction extends Hub.OAuthAction { // Pass back context to Looker if (response.status === 200) { const data = response.data; + const tokenPayload = new airtable_tokens_1.AirtableTokens(data.refresh_token, data.access_token, redirectUri); + const encrypted = await this.oauthMaybeEncryptTokens(tokenPayload, undefined); await gaxios.request({ url: payload.stateurl, method: "POST", - body: JSON.stringify({ tokens: { - refresh_token: data.refresh_token, - access_token: data.access_token, - }, redirect: redirectUri }), + body: encrypted, }).catch((_err) => { winston.error(_err.toString()); }); } else { diff --git a/lib/actions/airtable/airtable_tokens.d.ts b/lib/actions/airtable/airtable_tokens.d.ts new file mode 100644 index 000000000..e69414e2a --- /dev/null +++ b/lib/actions/airtable/airtable_tokens.d.ts @@ -0,0 +1,10 @@ +import { TokenPayload } from "../../hub"; +export declare class AirtableTokens extends TokenPayload { + static fromJson(json: any): AirtableTokens; + refresh_token: string; + access_token: string; + redirectUri?: string; + constructor(refreshToken: string, accessToken: string, redirectUri?: string); + asJson(): any; + toJSON(): any; +} diff --git a/lib/actions/airtable/airtable_tokens.js b/lib/actions/airtable/airtable_tokens.js new file mode 100644 index 000000000..a04188c12 --- /dev/null +++ b/lib/actions/airtable/airtable_tokens.js @@ -0,0 +1,31 @@ +"use strict"; +Object.defineProperty(exports, "__esModule", { value: true }); +exports.AirtableTokens = void 0; +const hub_1 = require("../../hub"); +class AirtableTokens extends hub_1.TokenPayload { + static fromJson(json) { + if (json.tokens) { + return new AirtableTokens(json.tokens.refresh_token, json.tokens.access_token, json.redirect); + } + return new AirtableTokens(json.refresh_token, json.access_token, json.redirectUri); + } + constructor(refreshToken, accessToken, redirectUri) { + super(); + this.refresh_token = refreshToken; + this.access_token = accessToken; + this.redirectUri = redirectUri; + } + asJson() { + return { + tokens: { + refresh_token: this.refresh_token, + access_token: this.access_token, + }, + redirect: this.redirectUri, + }; + } + toJSON() { + return this.asJson(); + } +} +exports.AirtableTokens = AirtableTokens; diff --git a/lib/actions/dropbox/dropbox.js b/lib/actions/dropbox/dropbox.js index 53b072516..1f5fb9d98 100644 --- a/lib/actions/dropbox/dropbox.js +++ b/lib/actions/dropbox/dropbox.js @@ -100,11 +100,9 @@ class DropboxAction extends Hub.OAuthAction { }], }]; if (accessToken !== "") { - if (accessToken !== "") { - const encrypted = await this.oauthMaybeEncryptTokens({ access_token: accessToken }, request.webhookId); - form.state = new Hub.ActionState(); - form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); - } + const encrypted = await this.oauthMaybeEncryptTokens({ access_token: accessToken }, request.webhookId); + form.state = new Hub.ActionState(); + form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); } return form; } diff --git a/lib/actions/salesforce/campaigns/salesforce_campaigns.js b/lib/actions/salesforce/campaigns/salesforce_campaigns.js index 7c87f6711..836619fd5 100644 --- a/lib/actions/salesforce/campaigns/salesforce_campaigns.js +++ b/lib/actions/salesforce/campaigns/salesforce_campaigns.js @@ -170,8 +170,8 @@ class SalesforceCampaignsAction extends Hub.OAuthAction { form.fields = fields; tokens = { access_token: sfdcConn.accessToken, refresh_token: sfdcConn.refreshToken }; form.state = new Hub.ActionState(); - const encrypted = await this.oauthMaybeEncryptTokens(tokens, request.webhookId); - form.state.data = typeof encrypted === "string" ? encrypted : JSON.stringify(encrypted); + const encryptedState = await this.oauthMaybeEncryptTokens(tokens, request.webhookId); + form.state.data = typeof encryptedState === "string" ? encryptedState : JSON.stringify(encryptedState); return form; } catch (e) {