diff --git a/.gitignore b/.gitignore index fdcf1d95b..e76e54373 100644 --- a/.gitignore +++ b/.gitignore @@ -63,4 +63,9 @@ typings/ .DS_Store # ActionHub specifics -status.json \ No newline at end of file +status.json + +# Internal Tooling +.agent/ +.gemini/ +tasks/ \ No newline at end of file diff --git a/lib/actions/airtable/airtable.js b/lib/actions/airtable/airtable.js index 5f3929bdf..2ddf404d2 100644 --- a/lib/actions/airtable/airtable.js +++ b/lib/actions/airtable/airtable.js @@ -52,11 +52,25 @@ class AirtableAction extends Hub.OAuthAction { let accessToken; let tokens; if (request.params.state_json) { - const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, 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); + const parsedState = JSON.parse(request.params.state_json); + if (parsedState.cid && parsedState.payload) { + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, 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); + } + else { + // Keeping the literal old code to ensure no regressions for unencrypted payloads + tokens = airtable_tokens_1.AirtableTokens.fromJson(parsedState); + accessToken = tokens.access_token; + state.data = JSON.stringify({ + tokens: { + refresh_token: tokens.refresh_token, + access_token: accessToken, + }, + }); + } } try { if (!accessToken) { @@ -104,14 +118,30 @@ class AirtableAction extends Hub.OAuthAction { }); } async form(request) { + // The form function handles the Airtable configuration form. + // It attempts to list the user's bases using the existing tokens in request.params.state_json. + // If listing succeeds, we return the base/table fields directly. + // if listing fails (e.g., token expired), we try to auto-refresh the token. + // If no tokens exist or refresh fails, it catches the error (in the outer catch block) + // and generates an OAuth link with a PKCE code_verifier to allow the user to re-authenticate. const form = new Hub.ActionForm(); try { let accessToken; let tokens; + let isEncrypted = false; if (request.params.state_json) { - const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); - tokens = airtable_tokens_1.AirtableTokens.fromJson(stateJson); - accessToken = tokens.access_token; + const parsedState = JSON.parse(request.params.state_json); + if (parsedState.cid && parsedState.payload) { + isEncrypted = true; + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); + tokens = airtable_tokens_1.AirtableTokens.fromJson(stateJson); + accessToken = tokens.access_token; + } + else { + // Keeping the literal old code to ensure no regressions for unencrypted payloads + tokens = airtable_tokens_1.AirtableTokens.fromJson(parsedState); + accessToken = tokens.access_token; + } } try { if (!accessToken) { @@ -120,12 +150,16 @@ class AirtableAction extends Hub.OAuthAction { await this.checkBaseList(accessToken); if (form.state === undefined) { form.state = new hub_1.ActionState(); - if (tokens) { + if (isEncrypted && 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; } + else { + // Keeping the literal old code to ensure no regressions for unencrypted payloads + form.state.data = request.params.state_json; + } } } catch (_a) { @@ -160,7 +194,8 @@ class AirtableAction extends Hub.OAuthAction { }]; } catch (e) { - // prevents others from impersonating you + // If no valid tokens exist (or refresh fail), we generate an OAuth link fallback. + // We create a code_verifier (PKCE) and encrypt it in the state payload to secure the exchange. 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 }); @@ -177,16 +212,26 @@ class AirtableAction extends Hub.OAuthAction { } return form; } + // oauthCheck determines if the user is authenticated by verifying that request.params.state_json + // contains valid (encrypted or unencrypted) token state. async oauthCheck(request) { if (request.params.state_json) { - const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); - if (stateJson) { - return true; + const parsedState = JSON.parse(request.params.state_json); + if (parsedState.cid && parsedState.payload) { + const stateJson = await this.oauthExtractTokensFromStateJson(request.params.state_json, request.webhookId); + return !!stateJson; + } + else { + // Keeping the literal old code to ensure no regressions for unencrypted payloads + const tokens = airtable_tokens_1.AirtableTokens.fromJson(parsedState); + return !!tokens.access_token; } } return false; } async oauthFetchInfo(urlParams, redirectUri) { + // oauthFetchInfo exchanges the authorization code for access and refresh tokens from Airtable. + // We decrypt the state to retrieve the stateurl and code_verifier. const actionCrypto = new Hub.ActionCrypto(); const plaintext = await actionCrypto.decrypt(urlParams.state).catch((err) => { winston.error("Encryption not correctly configured" + err); @@ -214,12 +259,14 @@ class AirtableAction extends Hub.OAuthAction { // Pass back context to Looker if (response.status === 200) { const data = response.data; + // Successful token exchange. Return tokens to Looker via the stateurl callback. const tokenPayload = new airtable_tokens_1.AirtableTokens(data.refresh_token, data.access_token, redirectUri); - const encrypted = await this.oauthMaybeEncryptTokens(tokenPayload, undefined); + // In this case we expect this function to return an encrypted token because AirTable is "enabled" for encryption. + const payloadWithEncryptedToken = await this.oauthMaybeEncryptTokens(tokenPayload, undefined); await gaxios.request({ url: payload.stateurl, method: "POST", - body: encrypted, + data: payloadWithEncryptedToken, }).catch((_err) => { winston.error(_err.toString()); }); } else { @@ -228,6 +275,7 @@ class AirtableAction extends Hub.OAuthAction { } } async oauthUrl(redirectUri, encryptedState) { + // oauthUrl constructs the authorization URL to redirect the user to Airtable's auth page. 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) => { diff --git a/lib/actions/airtable/airtable_tokens.js b/lib/actions/airtable/airtable_tokens.js index a04188c12..b352bb4a5 100644 --- a/lib/actions/airtable/airtable_tokens.js +++ b/lib/actions/airtable/airtable_tokens.js @@ -2,7 +2,12 @@ Object.defineProperty(exports, "__esModule", { value: true }); exports.AirtableTokens = void 0; const hub_1 = require("../../hub"); +// AirtableTokens holds the access and refresh tokens for Airtable OAuth. +// It extends TokenPayload to integrate with the Action Hub's OAuth framework. class AirtableTokens extends hub_1.TokenPayload { + // fromJson parses token payloads from two possible JSON shapes: + // 1. A nested shape (`{ tokens: { access_token, refresh_token }, redirect }`) used by newer ActionHub serialization. + // 2. A legacy flat shape (`{ access_token, refresh_token, redirectUri }`) used by older states. static fromJson(json) { if (json.tokens) { return new AirtableTokens(json.tokens.refresh_token, json.tokens.access_token, json.redirect); @@ -15,6 +20,8 @@ class AirtableTokens extends hub_1.TokenPayload { this.access_token = accessToken; this.redirectUri = redirectUri; } + // asJson serializes the tokens into a standard JSON shape that ActionHub expects. + // It matches the shape used by newer ActionHub serialization for encrypted payloads. asJson() { return { tokens: { diff --git a/lib/actions/dropbox/dropbox.d.ts b/lib/actions/dropbox/dropbox.d.ts index 4b1183433..042f16b31 100644 --- a/lib/actions/dropbox/dropbox.d.ts +++ b/lib/actions/dropbox/dropbox.d.ts @@ -18,6 +18,11 @@ export declare class DropboxAction extends Hub.OAuthAction { }, redirectUri: string): Promise; oauthCheck(request: Hub.ActionRequest): Promise; dropboxFilename(request: Hub.ActionRequest): string | undefined; + /** + * Exchanges the authorization code for an access token with Dropbox. + * Parameters are sent in the request body as application/x-www-form-urlencoded + * to comply with RFC 6749 and avoid leaking secrets in URL logs (b/426567813). + */ protected getAccessTokenFromCode(stateJson: any): Promise; protected dropboxClientFromRequest(request: Hub.ActionRequest, token: string): Promise; } diff --git a/lib/actions/dropbox/dropbox.js b/lib/actions/dropbox/dropbox.js index 27a5e8e01..eb83f0ce4 100644 --- a/lib/actions/dropbox/dropbox.js +++ b/lib/actions/dropbox/dropbox.js @@ -2,6 +2,7 @@ Object.defineProperty(exports, "__esModule", { value: true }); exports.DropboxAction = void 0; const dropbox_1 = require("dropbox"); +const gaxios = require("gaxios"); const querystring = require("querystring"); const https = require("request-promise-native"); const url_1 = require("url"); @@ -173,6 +174,7 @@ class DropboxAction extends Hub.OAuthAction { await https.post({ url: payload.stateurl, body: encrypted, + json: true, }).catch((_err) => { winston.error(_err.toString()); }); } // oauthCheck verifies if the Action Hub has a valid state for rendering or running. @@ -196,23 +198,37 @@ class DropboxAction extends Hub.OAuthAction { return request.formParams.filename; } } + /** + * Exchanges the authorization code for an access token with Dropbox. + * Parameters are sent in the request body as application/x-www-form-urlencoded + * to comply with RFC 6749 and avoid leaking secrets in URL logs (b/426567813). + */ async getAccessTokenFromCode(stateJson) { - const url = new url_1.URL("https://api.dropboxapi.com/oauth2/token"); + const url = "https://api.dropboxapi.com/oauth2/token"; if (stateJson.code && stateJson.redirect) { - url.search = querystring.stringify({ + const data = { grant_type: "authorization_code", code: stateJson.code, client_id: process.env.DROPBOX_ACTION_APP_KEY, client_secret: process.env.DROPBOX_ACTION_APP_SECRET, redirect_uri: stateJson.redirect, + }; + const response = await gaxios.request({ + method: "POST", + url, + data: querystring.stringify(data), + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + }).catch((err) => { + winston.error(`OAuth token exchange failed: ${err.message}`); + throw err; }); + return response.data.access_token; } else { throw "state_json does not contain correct members"; } - const response = await https.post(url.toString(), { json: true }) - .catch((_err) => { winston.error("Error requesting access_token"); }); - return response.access_token; } // dropboxClientFromRequest initializes a Dropbox client instance. // If token is defined, it uses it directly. Otherwise, it attempts to parse state_json to find an access_token. diff --git a/lib/hub/action_request.js b/lib/hub/action_request.js index c72102396..fb5639642 100644 --- a/lib/hub/action_request.js +++ b/lib/hub/action_request.js @@ -82,6 +82,18 @@ class ActionRequest { if (json.data) { request.params = json.data; } + if (json.state_url) { + if (request.params === undefined) { + request.params = {}; + } + request.params.state_url = json.state_url; + } + if (json.state_redir_url) { + if (request.params === undefined) { + request.params = {}; + } + request.params.state_redir_url = json.state_redir_url; + } if (json.form_params) { request.formParams = json.form_params; } diff --git a/lib/hub/oauth_action.d.ts b/lib/hub/oauth_action.d.ts index 186788229..bb162b01e 100644 --- a/lib/hub/oauth_action.d.ts +++ b/lib/hub/oauth_action.d.ts @@ -10,8 +10,19 @@ export declare abstract class OAuthAction extends Action { [key: string]: string; }, redirectUri: string): Promise; asJson(router: RouteBuilder, request: ActionRequest): any; + /** + * Extracts token state from a JSON string. + * - If parsing fails, returns null. + * - If encryption markers are present, attempts to decrypt. + * - Otherwise, returns the unencrypted JSON object. + */ oauthExtractTokensFromStateJson(stateJson: string, requestWebhookId: string | undefined): Promise; - oauthMaybeEncryptTokens(tokenPayload: any, requestWebhookId: string | undefined): Promise; + /** + * Conditionally encrypts token payloads based on the per-action environment config. + * - If `ENCRYPT_PAYLOAD_` is "true", returns an EncryptedPayload. + * - Otherwise, returns the raw JSON object so standard libraries like gaxios can set the correct Content-Type header. + */ + 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 393bc1417..397019138 100644 --- a/lib/hub/oauth_action.js +++ b/lib/hub/oauth_action.js @@ -12,6 +12,12 @@ class OAuthAction extends action_1.Action { json.uses_oauth = true; return json; } + /** + * Extracts token state from a JSON string. + * - If parsing fails, returns null. + * - If encryption markers are present, attempts to decrypt. + * - Otherwise, returns the unencrypted JSON object. + */ async oauthExtractTokensFromStateJson(stateJson, requestWebhookId) { let state; try { @@ -21,44 +27,39 @@ class OAuthAction extends action_1.Action { 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 encrypted_payload_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 { + if (!state.cid || !state.payload) { winston.info("Extracting unencrypted state_json", { webhookId: requestWebhookId, action: this.name }); return state; } + winston.info("Extracting encrypted state_json", { webhookId: requestWebhookId, action: this.name }); + const encryptedPayload = new encrypted_payload_1.EncryptedPayload(state.cid, state.payload); + try { + return await this.oauthDecryptTokens(encryptedPayload, requestWebhookId); + } + catch (e) { + winston.error(`Failed to decrypt or parse encrypted payload: ${e.message}`, { webhookId: requestWebhookId, action: this.name }); + return null; + } } + /** + * Conditionally encrypts token payloads based on the per-action environment config. + * - If `ENCRYPT_PAYLOAD_` is "true", returns an EncryptedPayload. + * - Otherwise, returns the raw JSON object so standard libraries like gaxios can set the correct Content-Type header. + */ 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 OAuthAction.actionCrypto.encrypt(JSON.stringify(tokenPayload)).catch((err) => { - winston.error("Encryption not correctly configured", { webhookId: requestWebhookId, action: this.name }); - throw err; - }); - const payload = new encrypted_payload_1.EncryptedPayload(encrypted_payload_1.EncryptedPayload.currentCipherId, encrypted); - return payload; - } - else { - return JSON.stringify(tokenPayload); + if (perActionEncryptionValue !== "true") { + return tokenPayload; } + const encrypted = await OAuthAction.actionCrypto.encrypt(JSON.stringify(tokenPayload)).catch((err) => { + winston.error("Encryption not correctly configured", { webhookId: requestWebhookId, action: this.name }); + throw err; + }); + return new encrypted_payload_1.EncryptedPayload(encrypted_payload_1.EncryptedPayload.currentCipherId, encrypted); } async oauthDecryptTokens(encryptedPayload, requestWebhookId) { + // This method decrypts the payload and validates the JSON shape. const jsonPayload = await OAuthAction.actionCrypto.decrypt(encryptedPayload.payload).catch((err) => { winston.error("Failed to decrypt state_json", { webhookId: requestWebhookId, action: this.name }); throw err; diff --git a/package.json b/package.json index 7b54b846b..d18c52d3a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "looker-action-hub", - "version": "1.5.31", + "version": "1.5.33", "description": "", "main": "lib/index", "scripts": { diff --git a/src/actions/airtable/test_airtable.ts b/src/actions/airtable/test_airtable.ts index 286f98703..b2e3e09ad 100644 --- a/src/actions/airtable/test_airtable.ts +++ b/src/actions/airtable/test_airtable.ts @@ -459,6 +459,40 @@ describe(`${action.constructor.name} unit tests`, () => { chai.expect(secondCallArgs.data).to.not.be.undefined // We want data to be used chai.expect(secondCallArgs.body).to.be.undefined // We want body to be unused }) + + it("sends an object payload instead of a string to ensure gaxios sets the correct Content-Type", async () => { + const urlParams = { + code: "test_code", + state: b64.encode(JSON.stringify({ + verifier: "test_verifier", + stateurl: "test_state_url", + })), + } + const redirectUri = "http://redirect" + + gaxiosStub.onFirstCall().resolves({ + status: 200, + data: { + access_token: "fresh_access", + refresh_token: "fresh_refresh", + }, + }) + + gaxiosStub.onSecondCall().resolves({ + status: 200, + data: {}, + }) + + // Ensure encryption is OFF to test the unencrypted fallback + delete process.env.ENCRYPT_PAYLOAD_AIRTABLE + await action.oauthFetchInfo(urlParams, redirectUri) + + const secondCallArgs = gaxiosStub.secondCall.args[0] + // This is the crucial assertion: If data is a string, gaxios will set Content-Type to text/plain. + // It MUST be an object for gaxios to set Content-Type to application/json. + chai.expect(typeof secondCallArgs.data).to.equal("object") + chai.expect(typeof secondCallArgs.data).to.not.equal("string") + }) }) }) }) diff --git a/src/actions/dropbox/dropbox.ts b/src/actions/dropbox/dropbox.ts index e8635c9a0..6ea55c8b6 100644 --- a/src/actions/dropbox/dropbox.ts +++ b/src/actions/dropbox/dropbox.ts @@ -1,4 +1,5 @@ import { Dropbox } from "dropbox" +import * as gaxios from "gaxios" import * as querystring from "querystring" import * as https from "request-promise-native" import {URL} from "url" @@ -171,6 +172,7 @@ export class DropboxAction extends Hub.OAuthAction { await https.post({ url: payload.stateurl, body: encrypted, + json: true, }).catch((_err) => { winston.error(_err.toString()) }) } @@ -195,23 +197,38 @@ export class DropboxAction extends Hub.OAuthAction { } } + /** + * Exchanges the authorization code for an access token with Dropbox. + * Parameters are sent in the request body as application/x-www-form-urlencoded + * to comply with RFC 6749 and avoid leaking secrets in URL logs (b/426567813). + */ protected async getAccessTokenFromCode(stateJson: any) { - const url = new URL("https://api.dropboxapi.com/oauth2/token") + const url = "https://api.dropboxapi.com/oauth2/token" if (stateJson.code && stateJson.redirect) { - url.search = querystring.stringify({ + const data = { grant_type: "authorization_code", code: stateJson.code, client_id: process.env.DROPBOX_ACTION_APP_KEY, client_secret: process.env.DROPBOX_ACTION_APP_SECRET, redirect_uri: stateJson.redirect, + } + + const response = await gaxios.request({ + method: "POST", + url, + data: querystring.stringify(data), + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + }).catch((err) => { + winston.error(`OAuth token exchange failed: ${err.message}`) + throw err }) + return response.data.access_token } else { throw "state_json does not contain correct members" } - const response = await https.post(url.toString(), { json: true }) - .catch((_err) => { winston.error("Error requesting access_token") }) - return response.access_token } // dropboxClientFromRequest initializes a Dropbox client instance. diff --git a/src/actions/dropbox/test_dropbox.ts b/src/actions/dropbox/test_dropbox.ts index 9288a811a..2fc669433 100644 --- a/src/actions/dropbox/test_dropbox.ts +++ b/src/actions/dropbox/test_dropbox.ts @@ -1,5 +1,6 @@ import * as b64 from "base64-url" import * as chai from "chai" +import * as gaxios from "gaxios" import * as https from "request-promise-native" import * as sinon from "sinon" @@ -251,11 +252,29 @@ describe(`${action.constructor.name} unit tests`, () => { `c2RmIiwiYXBwIjoibXlrZXkifQ`}, "redirect") chai.expect(stubEncrypt).to.have.been.calledWithMatch({code: "code", redirect: "redirect"}) - chai.expect(stubPost).to.have.been.calledWithMatch({body: "encrypted_state"}) + chai.expect(stubPost).to.have.been.calledWithMatch({body: "encrypted_state", json: true}) stubEncrypt.restore() stubPost.restore() }) + it("successfully uses body params in getAccessTokenFromCode", async () => { + const stubRequest = sinon.stub(gaxios, "request") + .resolves({data: {access_token: "body_token"}, status: 200} as any) + + try { + // @ts-ignore + const token = await action.getAccessTokenFromCode({code: "code", redirect: "redirect"}) + + chai.expect(token).to.equal("body_token") + chai.expect(stubRequest.calledOnce).to.be.true + chai.expect(stubRequest.calledWithMatch({ + method: "POST", + headers: {"Content-Type": "application/x-www-form-urlencoded"}, + })).to.be.true + } finally { + stubRequest.restore() + } + }) }) describe("dropboxFilename", () => { diff --git a/src/actions/facebook/test_facebook_custom_audiences.ts b/src/actions/facebook/test_facebook_custom_audiences.ts index 99b494a33..424c281db 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: JSON.stringify({ tokens: stubTokens, redirect: redirectUri }), + data: { tokens: stubTokens, redirect: redirectUri }, } await action.oauthFetchInfo({code: oauthCode, state: encryptedPayload}, redirectUri) diff --git a/src/actions/google/ads/test_customer_match.ts b/src/actions/google/ads/test_customer_match.ts index 5adc3fecf..70b66dad1 100644 --- a/src/actions/google/ads/test_customer_match.ts +++ b/src/actions/google/ads/test_customer_match.ts @@ -190,22 +190,22 @@ describe(`${action.constructor.name} class`, () => { const expectedPostArgs = { method: "POST", url: stateUrl, - data: JSON.stringify({ tokens: stubTokens, redirect: redirectUri }), + data: { tokens: stubTokens, redirect: redirectUri }, } await action.oauthFetchInfo({code: oauthCode, state: encryptedPayload}, redirectUri) - expect(oauthClientStub).to.be.calledOnce + expect(oauthClientStub.calledOnce).to.be.true expect(oauthClientStub.getCall(0).args).to.deep.equal([ "test_oauth_client_id", "test_oauth_client_secret", redirectUri, ]) - expect(getTokenStub).to.be.calledOnce + expect(getTokenStub.calledOnce).to.be.true expect(getTokenStub.getCall(0).args[0]).to.equal(oauthCode) - expect(gaxiosStub).to.be.calledOnce + expect(gaxiosStub.calledOnce).to.be.true expect(gaxiosStub.getCall(0).args[0]).to.deep.equal(expectedPostArgs) }) @@ -222,6 +222,37 @@ describe(`${action.constructor.name} class`, () => { it("wraps and re-throws any exceptions generated by the api") }) */ + + describe("state_url parsing bug reproduction", () => { + it("fails to pass stateUrl into payload if state_url is mapped to the root of JSON request", async () => { + // Looker Action API states state_url is generated per form request and sent inside the body root + const jsonPayload = { + type: "form", + webhookId: "test_webhook_id", + state_url: "https://looker.example.com/state_url_endpoint", + data: { + clientCid: testClientCid, + }, + } as any + + // This simulates action_request.ts mapping the JSON data + const request = Hub.ActionRequest.fromJSON(jsonPayload) + + // This simulates the OAuthHelper generating the login form payload + const form = await action.oauthHelper.makeLoginForm(request) + const authUrl = (form.fields[0] as any).oauth_url + + // The URL looks like BASE_URL/actions/action_name/oauth?state=ENCRYPTED_PAYLOAD + const urlParams = new URLSearchParams(authUrl.split("?")[1]) + const encrypted = urlParams.get("state")! + + // Inside actionCrypto, our test mock encodes as base64 instead of actual encrypt + const decrypted = b64.decode(encrypted) + const payload = JSON.parse(decrypted) + + expect(payload.stateUrl).to.equal("https://looker.example.com/state_url_endpoint") + }) + }) }) }) diff --git a/src/actions/google/analytics/test_data_import.ts b/src/actions/google/analytics/test_data_import.ts index 1643a8b5a..294d1553f 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: JSON.stringify({tokens: stubTokens, redirect: redirectUri}), + data: {tokens: stubTokens, redirect: redirectUri}, } await action.oauthFetchInfo({code: oauthCode, state: encryptedPayload}, redirectUri) diff --git a/src/hub/action_request.ts b/src/hub/action_request.ts index b9fac28f1..3aa05f0d1 100644 --- a/src/hub/action_request.ts +++ b/src/hub/action_request.ts @@ -136,6 +136,20 @@ export class ActionRequest { request.params = json.data } + if ((json as any).state_url) { + if (request.params === undefined) { + request.params = {} + } + request.params.state_url = (json as any).state_url + } + + if ((json as any).state_redir_url) { + if (request.params === undefined) { + request.params = {} + } + request.params.state_redir_url = (json as any).state_redir_url + } + if (json.form_params) { request.formParams = json.form_params } diff --git a/src/hub/oauth_action.ts b/src/hub/oauth_action.ts index 6a9294c1d..f72a8493a 100644 --- a/src/hub/oauth_action.ts +++ b/src/hub/oauth_action.ts @@ -58,17 +58,17 @@ export abstract class OAuthAction extends Action { /** * Conditionally encrypts token payloads based on the per-action environment config. * - If `ENCRYPT_PAYLOAD_` is "true", returns an EncryptedPayload. - * - Otherwise, returns a standard stringified JSON payload. + * - Otherwise, returns the raw JSON object so standard libraries like gaxios can set the correct Content-Type header. */ async oauthMaybeEncryptTokens( tokenPayload: any, requestWebhookId: string | undefined, - ): Promise { + ): Promise { const envVarName = `ENCRYPT_PAYLOAD_${this.name.toUpperCase()}` const perActionEncryptionValue = process.env[envVarName] if (perActionEncryptionValue !== "true") { - return JSON.stringify(tokenPayload) + return tokenPayload } const encrypted = await OAuthAction.actionCrypto.encrypt(JSON.stringify(tokenPayload)).catch((err: string) => { diff --git a/test/test_oauth_action.ts b/test/test_oauth_action.ts index 3e09b81ed..5eafb11fd 100644 --- a/test/test_oauth_action.ts +++ b/test/test_oauth_action.ts @@ -59,7 +59,7 @@ describe("OAuthAction Encryption", () => { process.env.ENCRYPT_PAYLOAD = "true" const payload = { tokens: "secret" } const result = await action.oauthMaybeEncryptTokens(payload, "webhookId") - chai.expect(result).to.equal(JSON.stringify(payload)) + chai.expect(result).to.deep.equal(payload) sinon.assert.notCalled(encryptStub) })