From 7807b734bb4f9768803f95b53350fe3611be9034 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 16 May 2024 14:41:45 +0200 Subject: [PATCH 1/6] Persist googlesheet refs in context --- packages/backend-core/src/context/types.ts | 6 ++ .../server/src/integrations/googlesheets.ts | 62 +++++++++++++------ 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/packages/backend-core/src/context/types.ts b/packages/backend-core/src/context/types.ts index f297d3089f..6694320978 100644 --- a/packages/backend-core/src/context/types.ts +++ b/packages/backend-core/src/context/types.ts @@ -1,4 +1,6 @@ import { IdentityContext, Snippet, VM } from "@budibase/types" +import { OAuth2Client } from "google-auth-library" +import { GoogleSpreadsheet } from "google-spreadsheet" // keep this out of Budibase types, don't want to expose context info export type ContextMap = { @@ -12,4 +14,8 @@ export type ContextMap = { vm?: VM cleanup?: (() => void | Promise)[] snippets?: Snippet[] + googleSheets?: { + oauthClient: OAuth2Client + clients: Record + } } diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index dc945b454a..28df55a981 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -228,32 +228,56 @@ class GoogleSheetsIntegration implements DatasourcePlus { private async connect() { try { - await setupCreationAuth(this.config) + const bbCtx = context.getCurrentContext() + let oauthClient = bbCtx?.googleSheets?.oauthClient - // Initialise oAuth client - const googleConfig = await configs.getGoogleDatasourceConfig() - if (!googleConfig) { - throw new HTTPError("Google config not found", 400) - } + if (!oauthClient) { + await setupCreationAuth(this.config) - const oauthClient = new OAuth2Client({ - clientId: googleConfig.clientID, - clientSecret: googleConfig.clientSecret, - }) + // Initialise oAuth client + const googleConfig = await configs.getGoogleDatasourceConfig() + if (!googleConfig) { + throw new HTTPError("Google config not found", 400) + } - const tokenResponse = await this.fetchAccessToken({ - client_id: googleConfig.clientID, - client_secret: googleConfig.clientSecret, - refresh_token: this.config.auth.refreshToken, - }) + oauthClient = new OAuth2Client({ + clientId: googleConfig.clientID, + clientSecret: googleConfig.clientSecret, + }) - oauthClient.setCredentials({ - refresh_token: this.config.auth.refreshToken, - access_token: tokenResponse.access_token, - }) + const tokenResponse = await this.fetchAccessToken({ + client_id: googleConfig.clientID, + client_secret: googleConfig.clientSecret, + refresh_token: this.config.auth.refreshToken, + }) + + oauthClient.setCredentials({ + refresh_token: this.config.auth.refreshToken, + access_token: tokenResponse.access_token, + }) + if (bbCtx && !bbCtx.googleSheets) { + bbCtx.googleSheets = { + oauthClient, + clients: {}, + } + bbCtx.cleanup = bbCtx.cleanup || [] this.client = new GoogleSpreadsheet(this.spreadsheetId, oauthClient) await this.client.loadInfo() + } + } + + let client = bbCtx?.googleSheets?.clients[this.spreadsheetId] + if (!client) { + client = new GoogleSpreadsheet(this.spreadsheetId, oauthClient) + await client.loadInfo() + + if (bbCtx?.googleSheets?.clients) { + bbCtx.googleSheets.clients[this.spreadsheetId] = client + } + } + + this.client = client } catch (err: any) { // this happens for xlsx imports if (err.message?.includes("operation is not supported")) { From a780a29337a219580f06870094a2de276d0f8928 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 16 May 2024 14:43:42 +0200 Subject: [PATCH 2/6] Clean --- packages/server/src/integrations/googlesheets.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index 28df55a981..d8a0dc9e65 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -261,9 +261,6 @@ class GoogleSheetsIntegration implements DatasourcePlus { clients: {}, } bbCtx.cleanup = bbCtx.cleanup || [] - - this.client = new GoogleSpreadsheet(this.spreadsheetId, oauthClient) - await this.client.loadInfo() } } From 39ad85127b838237c959ee6f1d3663b1d89e1db8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 17 May 2024 12:11:26 +0200 Subject: [PATCH 3/6] Use @budibase/google-spreadsheet --- packages/server/package.json | 2 +- yarn.lock | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/server/package.json b/packages/server/package.json index ab6738635d..e816ad3f18 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -79,7 +79,7 @@ "dotenv": "8.2.0", "form-data": "4.0.0", "global-agent": "3.0.0", - "google-spreadsheet": "4.1.2", + "google-spreadsheet": "npm:@budibase/google-spreadsheet@4.1.2", "ioredis": "5.3.2", "isolated-vm": "^4.7.2", "jimp": "0.22.10", diff --git a/yarn.lock b/yarn.lock index 810c7c2888..36ce2ce75e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11540,10 +11540,10 @@ google-p12-pem@^4.0.0: dependencies: node-forge "^1.3.1" -google-spreadsheet@4.1.2: +"google-spreadsheet@npm:@budibase/google-spreadsheet@4.1.2": version "4.1.2" - resolved "https://registry.yarnpkg.com/google-spreadsheet/-/google-spreadsheet-4.1.2.tgz#92e30fdba7e0d78c55d50731528df7835d58bfee" - integrity sha512-HFBweDAkOcyC2qO9kmWESKbNuOcn+R7UzZN/tj5LLNxVv8FHmg113u0Ow+yaKwwIOt/NnDtPLuptAhaxTs0FYw== + resolved "https://registry.yarnpkg.com/@budibase/google-spreadsheet/-/google-spreadsheet-4.1.2.tgz#90548ccba2284b3042b08d2974ef3caeaf772ad9" + integrity sha512-dxoY3rQGGnuNeZiXhNc9oYPduzU8xnIjWujFwNvaRRv3zWeUV7mj6HE2o/OJOeekPGt7o44B+w6DfkiaoteZgg== dependencies: axios "^1.4.0" lodash "^4.17.21" From ec2ec4014cc86304c087f6029bafee4644b5a4bd Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 17 May 2024 14:01:43 +0100 Subject: [PATCH 4/6] Fixing an issue with images and REST queries, these traditionally have only come back as binary data to Budibase, but this isn't very useful, its very difficult to convert these into something that can be used. Instead we will now download images into temporary attachments as we do for other types with a real content-disposition. --- packages/server/src/integrations/utils/restUtils.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/server/src/integrations/utils/restUtils.ts b/packages/server/src/integrations/utils/restUtils.ts index 2da9307071..8042cdba80 100644 --- a/packages/server/src/integrations/utils/restUtils.ts +++ b/packages/server/src/integrations/utils/restUtils.ts @@ -23,6 +23,15 @@ export function getAttachmentHeaders(headers: Headers) { } } } + // for images which don't supply a content disposition, make one up, as binary + // data for images in REST responses isn't really useful, we should always download them + else if (contentType.startsWith("image/")) { + const format = contentType.split("/")[1] + return { + contentDisposition: `attachment; filename="image.${format}"`, + contentType, + } + } return { contentDisposition, contentType } } From 16c69dcc33c4355b5e649b552ba9154b60e8cda2 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 17 May 2024 14:16:08 +0100 Subject: [PATCH 5/6] Backwards compat. --- .../ConfigEditor/stores/validatedConfig.js | 3 ++- packages/server/src/integrations/rest.ts | 23 ++++++++++++------- .../src/integrations/utils/restUtils.ts | 7 ++++-- .../types/src/documents/app/datasource.ts | 1 + 4 files changed, 23 insertions(+), 11 deletions(-) diff --git a/packages/builder/src/components/backend/Datasources/ConfigEditor/stores/validatedConfig.js b/packages/builder/src/components/backend/Datasources/ConfigEditor/stores/validatedConfig.js index 7b8b2c0975..edd39cc49f 100644 --- a/packages/builder/src/components/backend/Datasources/ConfigEditor/stores/validatedConfig.js +++ b/packages/builder/src/components/backend/Datasources/ConfigEditor/stores/validatedConfig.js @@ -86,8 +86,9 @@ export const createValidatedConfigStore = (integration, config) => { ([$configStore, $errorsStore, $selectedValidatorsStore]) => { const validatedConfig = [] + const allowedRestKeys = ["rejectUnauthorized", "downloadImages"] Object.entries(integration.datasource).forEach(([key, properties]) => { - if (integration.name === "REST" && key !== "rejectUnauthorized") { + if (integration.name === "REST" && !allowedRestKeys.includes(key)) { return } diff --git a/packages/server/src/integrations/rest.ts b/packages/server/src/integrations/rest.ts index 6ed8e4e4ec..7104c9daca 100644 --- a/packages/server/src/integrations/rest.ts +++ b/packages/server/src/integrations/rest.ts @@ -1,22 +1,22 @@ import { - Integration, DatasourceFieldType, - QueryType, - PaginationConfig, + HttpMethod, + Integration, IntegrationBase, + PaginationConfig, PaginationValues, - RestQueryFields as RestQuery, - RestConfig, + QueryType, RestAuthType, RestBasicAuthConfig, RestBearerAuthConfig, - HttpMethod, + RestConfig, + RestQueryFields as RestQuery, } from "@budibase/types" import get from "lodash/get" import * as https from "https" import qs from "querystring" -import fetch from "node-fetch" import type { Response } from "node-fetch" +import fetch from "node-fetch" import { formatBytes } from "../utilities" import { performance } from "perf_hooks" import FormData from "form-data" @@ -87,6 +87,12 @@ const SCHEMA: Integration = { default: true, required: false, }, + downloadImages: { + display: "Download images", + type: DatasourceFieldType.BOOLEAN, + default: true, + required: false, + }, }, query: { create: { @@ -139,7 +145,8 @@ class RestIntegration implements IntegrationBase { filename: string | undefined const { contentType, contentDisposition } = getAttachmentHeaders( - response.headers + response.headers, + { downloadImages: this.config.downloadImages } ) if ( contentDisposition.includes("filename") || diff --git a/packages/server/src/integrations/utils/restUtils.ts b/packages/server/src/integrations/utils/restUtils.ts index 8042cdba80..848cb7e33f 100644 --- a/packages/server/src/integrations/utils/restUtils.ts +++ b/packages/server/src/integrations/utils/restUtils.ts @@ -1,6 +1,9 @@ import type { Headers } from "node-fetch" -export function getAttachmentHeaders(headers: Headers) { +export function getAttachmentHeaders( + headers: Headers, + opts?: { downloadImages?: boolean } +) { const contentType = headers.get("content-type") || "" let contentDisposition = headers.get("content-disposition") || "" @@ -25,7 +28,7 @@ export function getAttachmentHeaders(headers: Headers) { } // for images which don't supply a content disposition, make one up, as binary // data for images in REST responses isn't really useful, we should always download them - else if (contentType.startsWith("image/")) { + else if (opts?.downloadImages && contentType.startsWith("image/")) { const format = contentType.split("/")[1] return { contentDisposition: `attachment; filename="image.${format}"`, diff --git a/packages/types/src/documents/app/datasource.ts b/packages/types/src/documents/app/datasource.ts index 32f5bbb132..e52019fc18 100644 --- a/packages/types/src/documents/app/datasource.ts +++ b/packages/types/src/documents/app/datasource.ts @@ -46,6 +46,7 @@ export interface DynamicVariable { export interface RestConfig { url: string rejectUnauthorized: boolean + downloadImages?: boolean defaultHeaders: { [key: string]: any } From e320524c6394272f41980f276222f803b6137b09 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 17 May 2024 14:37:01 +0100 Subject: [PATCH 6/6] Test case. --- .../src/integrations/tests/restUtils.spec.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/server/src/integrations/tests/restUtils.spec.ts b/packages/server/src/integrations/tests/restUtils.spec.ts index cdcaaec489..b26bd14c44 100644 --- a/packages/server/src/integrations/tests/restUtils.spec.ts +++ b/packages/server/src/integrations/tests/restUtils.spec.ts @@ -1,13 +1,13 @@ import { getAttachmentHeaders } from "../utils/restUtils" import type { Headers } from "node-fetch" -function headers(dispositionValue: string) { +function headers(dispositionValue: string, contentType?: string) { return { get: (name: string) => { if (name.toLowerCase() === "content-disposition") { return dispositionValue } else { - return "application/pdf" + return contentType || "application/pdf" } }, set: () => {}, @@ -35,4 +35,14 @@ describe("getAttachmentHeaders", () => { ) expect(contentDisposition).toBe(`inline; filename="report.pdf"`) }) + + it("should handle an image", () => { + const { contentDisposition } = getAttachmentHeaders( + headers("", "image/png"), + { + downloadImages: true, + } + ) + expect(contentDisposition).toBe(`attachment; filename="image.png"`) + }) })