From 36e1a20c03476fe7139982b46f86957ec8d2ba5c Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Thu, 15 Feb 2024 13:45:08 +0000 Subject: [PATCH 1/8] Revert "Revert "Fix updating users via cross-service comms (public API)"" --- .../server/src/utilities/workerRequests.ts | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/server/src/utilities/workerRequests.ts b/packages/server/src/utilities/workerRequests.ts index 69ce58c8a9..5612084216 100644 --- a/packages/server/src/utilities/workerRequests.ts +++ b/packages/server/src/utilities/workerRequests.ts @@ -14,12 +14,23 @@ export function request(ctx?: Ctx, request?: any) { if (!request.headers) { request.headers = {} } + if (!ctx) { request.headers[constants.Header.API_KEY] = coreEnv.INTERNAL_API_KEY - if (tenancy.isTenantIdSet()) { - request.headers[constants.Header.TENANT_ID] = tenancy.getTenantId() + } else if (ctx.headers) { + // copy all Budibase utilised headers over - copying everything can have + // side effects like requests being rejected due to odd content types etc + for (let header of Object.values(constants.Header)) { + if (ctx.headers[header]) { + request.headers[header] = ctx.headers[header] + } } } + + // apply tenancy if its available + if (tenancy.isTenantIdSet()) { + request.headers[constants.Header.TENANT_ID] = tenancy.getTenantId() + } if (request.body && Object.keys(request.body).length > 0) { request.headers["Content-Type"] = "application/json" request.body = @@ -29,9 +40,6 @@ export function request(ctx?: Ctx, request?: any) { } else { delete request.body } - if (ctx && ctx.headers) { - request.headers = ctx.headers - } // add x-budibase-correlation-id header logging.correlation.setHeader(request.headers) @@ -54,7 +62,7 @@ async function checkResponse( } const msg = `Unable to ${errorMsg} - ${responseErrorMessage}` if (ctx) { - ctx.throw(msg, response.status) + ctx.throw(response.status || 500, msg) } else { throw msg } From dde8f778774ab756edb51e1c4e2a208e80b70629 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 15 Feb 2024 14:48:47 +0000 Subject: [PATCH 2/8] Type workerRequests.ts --- .../src/logging/correlation/correlation.ts | 13 +++- .../server/src/utilities/workerRequests.ts | 78 ++++++++++++++----- 2 files changed, 70 insertions(+), 21 deletions(-) diff --git a/packages/backend-core/src/logging/correlation/correlation.ts b/packages/backend-core/src/logging/correlation/correlation.ts index 0498bd43d5..2ee02c16a9 100644 --- a/packages/backend-core/src/logging/correlation/correlation.ts +++ b/packages/backend-core/src/logging/correlation/correlation.ts @@ -1,10 +1,19 @@ +import { HeaderInit, Headers } from "node-fetch" import { Header } from "../../constants" const correlator = require("correlation-id") -export const setHeader = (headers: any) => { +export const setHeader = (headers: HeaderInit) => { const correlationId = correlator.getId() - if (correlationId) { + if (!correlationId) { + return + } + + if (headers instanceof Headers) { + headers.set(Header.CORRELATION_ID, correlationId) + } else if (Array.isArray(headers)) { + headers.push([Header.CORRELATION_ID, correlationId]) + } else { headers[Header.CORRELATION_ID] = correlationId } } diff --git a/packages/server/src/utilities/workerRequests.ts b/packages/server/src/utilities/workerRequests.ts index 5612084216..d3c5cad161 100644 --- a/packages/server/src/utilities/workerRequests.ts +++ b/packages/server/src/utilities/workerRequests.ts @@ -1,4 +1,10 @@ -import { Response, default as fetch } from "node-fetch" +import { + Response, + default as fetch, + type RequestInit, + Headers, + HeadersInit, +} from "node-fetch" import env from "../environment" import { checkSlashesInUrl } from "./index" import { @@ -10,29 +16,61 @@ import { } from "@budibase/backend-core" import { Ctx, User, EmailInvite } from "@budibase/types" -export function request(ctx?: Ctx, request?: any) { - if (!request.headers) { - request.headers = {} +function ensureHeadersIsObject(headers: HeadersInit | undefined): Headers { + if (headers instanceof Headers) { + return headers } + const headersObj = new Headers() + if (headers === undefined) { + return headersObj + } + + if (Array.isArray(headers)) { + for (const [key, value] of headers) { + headersObj.append(key, value) + } + } else { + for (const key in headers) { + headersObj.append(key, headers[key]) + } + } + return headersObj +} + +export function request(request: RequestInit & { ctx?: Ctx }): RequestInit { + const ctx = request.ctx + request.headers = ensureHeadersIsObject(request.headers) + if (!ctx) { - request.headers[constants.Header.API_KEY] = coreEnv.INTERNAL_API_KEY + if (coreEnv.INTERNAL_API_KEY) { + request.headers.set(constants.Header.API_KEY, coreEnv.INTERNAL_API_KEY) + } } else if (ctx.headers) { // copy all Budibase utilised headers over - copying everything can have // side effects like requests being rejected due to odd content types etc for (let header of Object.values(constants.Header)) { - if (ctx.headers[header]) { - request.headers[header] = ctx.headers[header] + const value = ctx.headers[header] + if (value === undefined) { + continue + } + + if (Array.isArray(value)) { + for (let v of value) { + request.headers.append(header, v) + } + } else { + request.headers.set(header, value) } } } // apply tenancy if its available if (tenancy.isTenantIdSet()) { - request.headers[constants.Header.TENANT_ID] = tenancy.getTenantId() + request.headers.set(constants.Header.TENANT_ID, tenancy.getTenantId()) } if (request.body && Object.keys(request.body).length > 0) { - request.headers["Content-Type"] = "application/json" + request.headers.set("Content-Type", "application/json") request.body = typeof request.body === "object" ? JSON.stringify(request.body) @@ -44,6 +82,7 @@ export function request(ctx?: Ctx, request?: any) { // add x-budibase-correlation-id header logging.correlation.setHeader(request.headers) + delete request.ctx return request } @@ -93,9 +132,9 @@ export async function sendSmtpEmail({ // tenant ID will be set in header const response = await fetch( checkSlashesInUrl(env.WORKER_URL + `/api/global/email/send`), - request(undefined, { + request({ method: "POST", - body: { + body: JSON.stringify({ email: to, from, contents, @@ -105,7 +144,7 @@ export async function sendSmtpEmail({ purpose: "custom", automation, invite, - }, + }), }) ) return checkResponse(response, "send email") @@ -115,7 +154,8 @@ export async function removeAppFromUserRoles(ctx: Ctx, appId: string) { const prodAppId = dbCore.getProdAppID(appId) const response = await fetch( checkSlashesInUrl(env.WORKER_URL + `/api/global/roles/${prodAppId}`), - request(ctx, { + request({ + ctx, method: "DELETE", }) ) @@ -126,7 +166,7 @@ export async function allGlobalUsers(ctx: Ctx) { const response = await fetch( checkSlashesInUrl(env.WORKER_URL + "/api/global/users"), // we don't want to use API key when getting self - request(ctx, { method: "GET" }) + request({ ctx, method: "GET" }) ) return checkResponse(response, "get users", { ctx }) } @@ -135,7 +175,7 @@ export async function saveGlobalUser(ctx: Ctx) { const response = await fetch( checkSlashesInUrl(env.WORKER_URL + "/api/global/users"), // we don't want to use API key when getting self - request(ctx, { method: "POST", body: ctx.request.body }) + request({ ctx, method: "POST", body: ctx.request.body }) ) return checkResponse(response, "save user", { ctx }) } @@ -146,7 +186,7 @@ export async function deleteGlobalUser(ctx: Ctx) { env.WORKER_URL + `/api/global/users/${ctx.params.userId}` ), // we don't want to use API key when getting self - request(ctx, { method: "DELETE" }) + request({ ctx, method: "DELETE" }) ) return checkResponse(response, "delete user", { ctx }) } @@ -157,7 +197,7 @@ export async function readGlobalUser(ctx: Ctx): Promise { env.WORKER_URL + `/api/global/users/${ctx.params.userId}` ), // we don't want to use API key when getting self - request(ctx, { method: "GET" }) + request({ ctx, method: "GET" }) ) return checkResponse(response, "get user", { ctx }) } @@ -167,7 +207,7 @@ export async function getChecklist(): Promise<{ }> { const response = await fetch( checkSlashesInUrl(env.WORKER_URL + "/api/global/configs/checklist"), - request(undefined, { method: "GET" }) + request({ method: "GET" }) ) return checkResponse(response, "get checklist") } @@ -175,7 +215,7 @@ export async function getChecklist(): Promise<{ export async function generateApiKey(userId: string) { const response = await fetch( checkSlashesInUrl(env.WORKER_URL + "/api/global/self/api_key"), - request(undefined, { method: "POST", body: { userId } }) + request({ method: "POST", body: JSON.stringify({ userId }) }) ) return checkResponse(response, "generate API key") } From 1f4a254ec5449e2e04797ce3fa904cf14048ccde Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 15 Feb 2024 15:47:56 +0000 Subject: [PATCH 3/8] Fix for integration test, make sure to carry auth headers over correctly. --- packages/server/src/utilities/workerRequests.ts | 9 +++++++++ packages/shared-core/src/constants/api.ts | 1 + 2 files changed, 10 insertions(+) diff --git a/packages/server/src/utilities/workerRequests.ts b/packages/server/src/utilities/workerRequests.ts index 5612084216..b5c3bfcd4a 100644 --- a/packages/server/src/utilities/workerRequests.ts +++ b/packages/server/src/utilities/workerRequests.ts @@ -7,6 +7,7 @@ import { tenancy, logging, env as coreEnv, + utils, } from "@budibase/backend-core" import { Ctx, User, EmailInvite } from "@budibase/types" @@ -25,6 +26,14 @@ export function request(ctx?: Ctx, request?: any) { request.headers[header] = ctx.headers[header] } } + // be specific about auth headers + const cookie = ctx.headers[constants.Header.COOKIE], + apiKey = ctx.headers[constants.Header.API_KEY] + if (cookie) { + request.headers[constants.Header.COOKIE] = cookie + } else if (apiKey) { + request.headers[constants.Header.API_KEY] = apiKey + } } // apply tenancy if its available diff --git a/packages/shared-core/src/constants/api.ts b/packages/shared-core/src/constants/api.ts index d6633649e6..f63849bf3d 100644 --- a/packages/shared-core/src/constants/api.ts +++ b/packages/shared-core/src/constants/api.ts @@ -16,4 +16,5 @@ export enum Header { CORRELATION_ID = "x-budibase-correlation-id", AUTHORIZATION = "authorization", MIGRATING_APP = "x-budibase-migrating-app", + COOKIE = "cookie", } From e4b03308664f5d0054a7ca94fb55b9e89f92e8cf Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 15 Feb 2024 15:49:30 +0000 Subject: [PATCH 4/8] Simplify the typing of workerRequests.ts --- .../src/logging/correlation/correlation.ts | 12 +--- .../server/src/utilities/workerRequests.ts | 70 +++++++++---------- 2 files changed, 37 insertions(+), 45 deletions(-) diff --git a/packages/backend-core/src/logging/correlation/correlation.ts b/packages/backend-core/src/logging/correlation/correlation.ts index 2ee02c16a9..13cc7aff8f 100644 --- a/packages/backend-core/src/logging/correlation/correlation.ts +++ b/packages/backend-core/src/logging/correlation/correlation.ts @@ -1,21 +1,13 @@ -import { HeaderInit, Headers } from "node-fetch" import { Header } from "../../constants" const correlator = require("correlation-id") -export const setHeader = (headers: HeaderInit) => { +export const setHeader = (headers: Record) => { const correlationId = correlator.getId() if (!correlationId) { return } - - if (headers instanceof Headers) { - headers.set(Header.CORRELATION_ID, correlationId) - } else if (Array.isArray(headers)) { - headers.push([Header.CORRELATION_ID, correlationId]) - } else { - headers[Header.CORRELATION_ID] = correlationId - } + headers[Header.CORRELATION_ID] = correlationId } export function getId() { diff --git a/packages/server/src/utilities/workerRequests.ts b/packages/server/src/utilities/workerRequests.ts index d3c5cad161..ce43c1aa44 100644 --- a/packages/server/src/utilities/workerRequests.ts +++ b/packages/server/src/utilities/workerRequests.ts @@ -38,15 +38,24 @@ function ensureHeadersIsObject(headers: HeadersInit | undefined): Headers { return headersObj } -export function request(request: RequestInit & { ctx?: Ctx }): RequestInit { - const ctx = request.ctx - request.headers = ensureHeadersIsObject(request.headers) +interface Request { + ctx?: Ctx + method: "GET" | "POST" | "PUT" | "DELETE" | "PATCH" + headers?: { [key: string]: string } + body?: { [key: string]: any } +} - if (!ctx) { - if (coreEnv.INTERNAL_API_KEY) { - request.headers.set(constants.Header.API_KEY, coreEnv.INTERNAL_API_KEY) - } - } else if (ctx.headers) { +export function createRequest(request: Request): RequestInit { + const headers: Record = {} + const requestInit: RequestInit = { + method: request.method, + } + + const ctx = request.ctx + + if (!ctx && coreEnv.INTERNAL_API_KEY) { + headers[constants.Header.API_KEY] = coreEnv.INTERNAL_API_KEY + } else if (ctx && ctx.headers) { // copy all Budibase utilised headers over - copying everything can have // side effects like requests being rejected due to odd content types etc for (let header of Object.values(constants.Header)) { @@ -56,34 +65,25 @@ export function request(request: RequestInit & { ctx?: Ctx }): RequestInit { } if (Array.isArray(value)) { - for (let v of value) { - request.headers.append(header, v) - } + headers[header] = value[0] } else { - request.headers.set(header, value) + headers[header] = value } } } // apply tenancy if its available if (tenancy.isTenantIdSet()) { - request.headers.set(constants.Header.TENANT_ID, tenancy.getTenantId()) + headers[constants.Header.TENANT_ID] = tenancy.getTenantId() } + if (request.body && Object.keys(request.body).length > 0) { - request.headers.set("Content-Type", "application/json") - request.body = - typeof request.body === "object" - ? JSON.stringify(request.body) - : request.body - } else { - delete request.body + headers["Content-Type"] = "application/json" + requestInit.body = JSON.stringify(request.body) } - // add x-budibase-correlation-id header - logging.correlation.setHeader(request.headers) - - delete request.ctx - return request + logging.correlation.setHeader(headers) + return requestInit } async function checkResponse( @@ -132,9 +132,9 @@ export async function sendSmtpEmail({ // tenant ID will be set in header const response = await fetch( checkSlashesInUrl(env.WORKER_URL + `/api/global/email/send`), - request({ + createRequest({ method: "POST", - body: JSON.stringify({ + body: { email: to, from, contents, @@ -144,7 +144,7 @@ export async function sendSmtpEmail({ purpose: "custom", automation, invite, - }), + }, }) ) return checkResponse(response, "send email") @@ -154,7 +154,7 @@ export async function removeAppFromUserRoles(ctx: Ctx, appId: string) { const prodAppId = dbCore.getProdAppID(appId) const response = await fetch( checkSlashesInUrl(env.WORKER_URL + `/api/global/roles/${prodAppId}`), - request({ + createRequest({ ctx, method: "DELETE", }) @@ -166,7 +166,7 @@ export async function allGlobalUsers(ctx: Ctx) { const response = await fetch( checkSlashesInUrl(env.WORKER_URL + "/api/global/users"), // we don't want to use API key when getting self - request({ ctx, method: "GET" }) + createRequest({ ctx, method: "GET" }) ) return checkResponse(response, "get users", { ctx }) } @@ -175,7 +175,7 @@ export async function saveGlobalUser(ctx: Ctx) { const response = await fetch( checkSlashesInUrl(env.WORKER_URL + "/api/global/users"), // we don't want to use API key when getting self - request({ ctx, method: "POST", body: ctx.request.body }) + createRequest({ ctx, method: "POST", body: ctx.request.body }) ) return checkResponse(response, "save user", { ctx }) } @@ -186,7 +186,7 @@ export async function deleteGlobalUser(ctx: Ctx) { env.WORKER_URL + `/api/global/users/${ctx.params.userId}` ), // we don't want to use API key when getting self - request({ ctx, method: "DELETE" }) + createRequest({ ctx, method: "DELETE" }) ) return checkResponse(response, "delete user", { ctx }) } @@ -197,7 +197,7 @@ export async function readGlobalUser(ctx: Ctx): Promise { env.WORKER_URL + `/api/global/users/${ctx.params.userId}` ), // we don't want to use API key when getting self - request({ ctx, method: "GET" }) + createRequest({ ctx, method: "GET" }) ) return checkResponse(response, "get user", { ctx }) } @@ -207,7 +207,7 @@ export async function getChecklist(): Promise<{ }> { const response = await fetch( checkSlashesInUrl(env.WORKER_URL + "/api/global/configs/checklist"), - request({ method: "GET" }) + createRequest({ method: "GET" }) ) return checkResponse(response, "get checklist") } @@ -215,7 +215,7 @@ export async function getChecklist(): Promise<{ export async function generateApiKey(userId: string) { const response = await fetch( checkSlashesInUrl(env.WORKER_URL + "/api/global/self/api_key"), - request({ method: "POST", body: JSON.stringify({ userId }) }) + createRequest({ method: "POST", body: { userId } }) ) return checkResponse(response, "generate API key") } From b4669b32f00158e72b637cf13747c5b2a26618f6 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 15 Feb 2024 15:52:06 +0000 Subject: [PATCH 5/8] Fix build. --- packages/server/src/api/controllers/dev.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/controllers/dev.ts b/packages/server/src/api/controllers/dev.ts index d8c26725f1..497da088c6 100644 --- a/packages/server/src/api/controllers/dev.ts +++ b/packages/server/src/api/controllers/dev.ts @@ -1,7 +1,7 @@ import fetch from "node-fetch" import env from "../../environment" import { checkSlashesInUrl } from "../../utilities" -import { request } from "../../utilities/workerRequests" +import { createRequest } from "../../utilities/workerRequests" import { clearLock as redisClearLock } from "../../utilities/redis" import { DocumentType } from "../../db/utils" import { @@ -13,14 +13,19 @@ import { } from "@budibase/backend-core" import { App } from "@budibase/types" -async function redirect(ctx: any, method: string, path: string = "global") { +async function redirect( + ctx: any, + method: "GET" | "POST" | "DELETE", + path: string = "global" +) { const { devPath } = ctx.params const queryString = ctx.originalUrl.split("?")[1] || "" const response = await fetch( checkSlashesInUrl( `${env.WORKER_URL}/api/${path}/${devPath}?${queryString}` ), - request(ctx, { + createRequest({ + ctx, method, body: ctx.request.body, }) From c2c0108e4fd7f6595f7ddda547b46f713fa301c3 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 15 Feb 2024 16:12:47 +0000 Subject: [PATCH 6/8] Fix build (again). --- packages/server/src/utilities/workerRequests.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/server/src/utilities/workerRequests.ts b/packages/server/src/utilities/workerRequests.ts index acf7515784..def3ffe9cf 100644 --- a/packages/server/src/utilities/workerRequests.ts +++ b/packages/server/src/utilities/workerRequests.ts @@ -75,9 +75,13 @@ export function createRequest(request: Request): RequestInit { const cookie = ctx.headers[constants.Header.COOKIE], apiKey = ctx.headers[constants.Header.API_KEY] if (cookie) { - request.headers[constants.Header.COOKIE] = cookie + headers[constants.Header.COOKIE] = cookie } else if (apiKey) { - request.headers[constants.Header.API_KEY] = apiKey + if (Array.isArray(apiKey)) { + headers[constants.Header.API_KEY] = apiKey[0] + } else { + headers[constants.Header.API_KEY] = apiKey + } } } From 32815d8d9bec6c1efe58f4fcdcd226b1e47364ce Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 15 Feb 2024 16:28:59 +0000 Subject: [PATCH 7/8] Quick readability enhancement. --- packages/server/src/utilities/workerRequests.ts | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/packages/server/src/utilities/workerRequests.ts b/packages/server/src/utilities/workerRequests.ts index def3ffe9cf..054514ab59 100644 --- a/packages/server/src/utilities/workerRequests.ts +++ b/packages/server/src/utilities/workerRequests.ts @@ -64,12 +64,7 @@ export function createRequest(request: Request): RequestInit { if (value === undefined) { continue } - - if (Array.isArray(value)) { - headers[header] = value[0] - } else { - headers[header] = value - } + headers[header] = Array.isArray(value) ? value[0] : value } // be specific about auth headers const cookie = ctx.headers[constants.Header.COOKIE], @@ -77,11 +72,9 @@ export function createRequest(request: Request): RequestInit { if (cookie) { headers[constants.Header.COOKIE] = cookie } else if (apiKey) { - if (Array.isArray(apiKey)) { - headers[constants.Header.API_KEY] = apiKey[0] - } else { - headers[constants.Header.API_KEY] = apiKey - } + headers[constants.Header.API_KEY] = Array.isArray(apiKey) + ? apiKey[0] + : apiKey } } From dd4ea4be95c733636f42ed8161f395c2217699f8 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 15 Feb 2024 16:44:19 +0000 Subject: [PATCH 8/8] Nothing to see here, carry on. --- .../server/src/utilities/workerRequests.ts | 23 +------------------ 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/packages/server/src/utilities/workerRequests.ts b/packages/server/src/utilities/workerRequests.ts index 054514ab59..91340c8d78 100644 --- a/packages/server/src/utilities/workerRequests.ts +++ b/packages/server/src/utilities/workerRequests.ts @@ -17,28 +17,6 @@ import { } from "@budibase/backend-core" import { Ctx, User, EmailInvite } from "@budibase/types" -function ensureHeadersIsObject(headers: HeadersInit | undefined): Headers { - if (headers instanceof Headers) { - return headers - } - - const headersObj = new Headers() - if (headers === undefined) { - return headersObj - } - - if (Array.isArray(headers)) { - for (const [key, value] of headers) { - headersObj.append(key, value) - } - } else { - for (const key in headers) { - headersObj.append(key, headers[key]) - } - } - return headersObj -} - interface Request { ctx?: Ctx method: "GET" | "POST" | "PUT" | "DELETE" | "PATCH" @@ -89,6 +67,7 @@ export function createRequest(request: Request): RequestInit { } logging.correlation.setHeader(headers) + requestInit.headers = headers return requestInit }