From 3fc170c16bd1a99848cc260c94440f381497e4e2 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 15 Oct 2024 17:33:32 +0100 Subject: [PATCH 01/38] Add settings to automation context. --- .../src/api/routes/tests/automation.spec.ts | 66 +++++++++++++++++-- .../tests/utilities/AutomationTestBuilder.ts | 7 +- .../server/src/definitions/automations.ts | 5 ++ packages/server/src/threads/automation.ts | 9 ++- 4 files changed, 80 insertions(+), 7 deletions(-) diff --git a/packages/server/src/api/routes/tests/automation.spec.ts b/packages/server/src/api/routes/tests/automation.spec.ts index b6233b24ad..a517346c82 100644 --- a/packages/server/src/api/routes/tests/automation.spec.ts +++ b/packages/server/src/api/routes/tests/automation.spec.ts @@ -9,9 +9,15 @@ import { TRIGGER_DEFINITIONS, BUILTIN_ACTION_DEFINITIONS, } from "../../../automations" -import { events } from "@budibase/backend-core" +import { configs, context, events } from "@budibase/backend-core" import sdk from "../../../sdk" -import { Automation, FieldType, Table } from "@budibase/types" +import { + Automation, + ConfigType, + FieldType, + SettingsConfig, + Table, +} from "@budibase/types" import { mocks } from "@budibase/backend-core/tests" import { FilterConditions } from "../../../automations/steps/filter" import { removeDeprecated } from "../../../automations/utils" @@ -39,8 +45,7 @@ describe("/automations", () => { }) beforeEach(() => { - // @ts-ignore - events.automation.deleted.mockClear() + jest.clearAllMocks() }) describe("get definitions", () => { @@ -244,6 +249,59 @@ describe("/automations", () => { }) }) + describe("run", () => { + let oldConfig: SettingsConfig + beforeAll(async () => { + await context.doInTenant(config.getTenantId(), async () => { + oldConfig = await configs.getSettingsConfigDoc() + + const settings: SettingsConfig = { + _id: oldConfig._id, + _rev: oldConfig._rev, + type: ConfigType.SETTINGS, + config: { + platformUrl: "https://example.com", + logoUrl: "https://example.com/logo.png", + company: "Test Company", + }, + } + const saved = await configs.save(settings) + oldConfig._rev = saved.rev + }) + }) + + afterAll(async () => { + await context.doInTenant(config.getTenantId(), async () => { + await configs.save(oldConfig) + }) + }) + + it("should be able to access platformUrl, logoUrl and company in the automation", async () => { + const result = await createAutomationBuilder({ + name: "Test Automation", + appId: config.getAppId(), + config, + }) + .appAction({ fields: {} }) + .serverLog({ + text: "{{ settings.url }}", + }) + .serverLog({ + text: "{{ settings.logo }}", + }) + .serverLog({ + text: "{{ settings.company }}", + }) + .run() + + expect(result.steps[0].outputs.message).toEndWith("https://example.com") + expect(result.steps[1].outputs.message).toEndWith( + "https://example.com/logo.png" + ) + expect(result.steps[2].outputs.message).toEndWith("Test Company") + }) + }) + describe("test", () => { it("tests the automation successfully", async () => { let table = await config.createTable() diff --git a/packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts b/packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts index 6aaf22cd6a..fa8061edce 100644 --- a/packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts +++ b/packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts @@ -225,7 +225,9 @@ class AutomationBuilder extends BaseStepBuilder { private triggerOutputs: any private triggerSet: boolean = false - constructor(options: { name?: string; appId?: string } = {}) { + constructor( + options: { name?: string; appId?: string; config?: TestConfiguration } = {} + ) { super() this.automationConfig = { name: options.name || `Test Automation ${uuidv4()}`, @@ -237,7 +239,7 @@ class AutomationBuilder extends BaseStepBuilder { type: "automation", appId: options.appId ?? setup.getConfig().getAppId(), } - this.config = setup.getConfig() + this.config = options.config || setup.getConfig() } // TRIGGERS @@ -347,6 +349,7 @@ class AutomationBuilder extends BaseStepBuilder { export function createAutomationBuilder(options?: { name?: string appId?: string + config?: TestConfiguration }) { return new AutomationBuilder(options) } diff --git a/packages/server/src/definitions/automations.ts b/packages/server/src/definitions/automations.ts index 9433075da7..afee81ee0a 100644 --- a/packages/server/src/definitions/automations.ts +++ b/packages/server/src/definitions/automations.ts @@ -19,4 +19,9 @@ export interface AutomationContext extends AutomationResults { stepsByName: Record env?: Record trigger: any + settings?: { + url?: string + logo?: string + company?: string + } } diff --git a/packages/server/src/threads/automation.ts b/packages/server/src/threads/automation.ts index 3b47634663..f7db52ec1a 100644 --- a/packages/server/src/threads/automation.ts +++ b/packages/server/src/threads/automation.ts @@ -29,7 +29,7 @@ import { } from "@budibase/types" import { AutomationContext, TriggerOutput } from "../definitions/automations" import { WorkerCallback } from "./definitions" -import { context, logging } from "@budibase/backend-core" +import { context, logging, configs } from "@budibase/backend-core" import { processObject, processStringSync } from "@budibase/string-templates" import { cloneDeep } from "lodash/fp" import { performance } from "perf_hooks" @@ -259,6 +259,13 @@ class Orchestrator { }) this.context.env = await sdkUtils.getEnvironmentVariables() + const { config } = await configs.getSettingsConfigDoc() + this.context.settings = { + url: config.platformUrl, + logo: config.logoUrl, + company: config.company, + } + let metadata // check if this is a recurring automation, From 71671ad62ab7735bcb739554b9f90a76a1829d96 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 23 Oct 2024 14:24:24 +0200 Subject: [PATCH 02/38] Update pro submodule --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 297fdc937e..6041196305 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 297fdc937e9c650b4964fc1a942b60022b195865 +Subproject commit 60411963053b98e0415a1e9285b721fc26c628c8 From 67d2e0cf61dfd09df3be98ecb13be6dbe3300d6e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 23 Oct 2024 14:34:39 +0200 Subject: [PATCH 03/38] Port changes from PR #14846 --- .../src/api/controllers/global/configs.ts | 56 +++++++++++-------- .../controllers/global/tests/configs.spec.ts | 4 -- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/packages/worker/src/api/controllers/global/configs.ts b/packages/worker/src/api/controllers/global/configs.ts index e6e80ff3a5..750b02088c 100644 --- a/packages/worker/src/api/controllers/global/configs.ts +++ b/packages/worker/src/api/controllers/global/configs.ts @@ -44,9 +44,7 @@ const getEventFns = async (config: Config, existing?: Config) => { fns.push(events.email.SMTPCreated) } else if (isAIConfig(config)) { fns.push(() => events.ai.AIConfigCreated) - fns.push(() => - pro.quotas.updateCustomAIConfigCount(Object.keys(config.config).length) - ) + fns.push(() => pro.quotas.addCustomAIConfig()) } else if (isGoogleConfig(config)) { fns.push(() => events.auth.SSOCreated(ConfigType.GOOGLE)) if (config.config.activated) { @@ -85,9 +83,6 @@ const getEventFns = async (config: Config, existing?: Config) => { fns.push(events.email.SMTPUpdated) } else if (isAIConfig(config)) { fns.push(() => events.ai.AIConfigUpdated) - fns.push(() => - pro.quotas.updateCustomAIConfigCount(Object.keys(config.config).length) - ) } else if (isGoogleConfig(config)) { fns.push(() => events.auth.SSOUpdated(ConfigType.GOOGLE)) if (!existing.config.activated && config.config.activated) { @@ -253,7 +248,7 @@ export async function save(ctx: UserCtx) { if (existingConfig) { await verifyAIConfig(config, existingConfig) } - await pro.quotas.updateCustomAIConfigCount(Object.keys(config).length) + await pro.quotas.addCustomAIConfig() break } } catch (err: any) { @@ -342,29 +337,43 @@ export async function find(ctx: UserCtx) { let scopedConfig = await configs.getConfig(type) if (scopedConfig) { - if (type === ConfigType.OIDC_LOGOS) { - enrichOIDCLogos(scopedConfig) - } - - if (type === ConfigType.AI) { - await pro.sdk.ai.enrichAIConfig(scopedConfig) - // Strip out the API Keys from the response so they don't show in the UI - for (const key in scopedConfig.config) { - if (scopedConfig.config[key].apiKey) { - scopedConfig.config[key].apiKey = PASSWORD_REPLACEMENT - } - } - } - ctx.body = scopedConfig + await handleConfigType(type, scopedConfig) + } else if (type === ConfigType.AI) { + scopedConfig = { config: {} } as AIConfig + await handleAIConfig(scopedConfig) } else { - // don't throw an error, there simply is nothing to return + // If no config found and not AI type, just return an empty body ctx.body = {} + return } + + ctx.body = scopedConfig } catch (err: any) { ctx.throw(err?.status || 400, err) } } +async function handleConfigType(type: ConfigType, config: Config) { + if (type === ConfigType.OIDC_LOGOS) { + enrichOIDCLogos(config) + } else if (type === ConfigType.AI) { + await handleAIConfig(config) + } +} + +async function handleAIConfig(config: AIConfig) { + await pro.sdk.ai.enrichAIConfig(config) + stripApiKeys(config) +} + +function stripApiKeys(config: AIConfig) { + for (const key in config?.config) { + if (config.config[key].apiKey) { + config.config[key].apiKey = PASSWORD_REPLACEMENT + } + } +} + export async function publicOidc(ctx: Ctx) { try { // Find the config with the most granular scope based on context @@ -508,6 +517,9 @@ export async function destroy(ctx: UserCtx) { try { await db.remove(id, rev) await cache.destroy(cache.CacheKey.CHECKLIST) + if (id === configs.generateConfigID(ConfigType.AI)) { + await pro.quotas.removeCustomAIConfig() + } ctx.body = { message: "Config deleted successfully" } } catch (err: any) { ctx.throw(err.status, err) diff --git a/packages/worker/src/api/controllers/global/tests/configs.spec.ts b/packages/worker/src/api/controllers/global/tests/configs.spec.ts index 9091f29247..857f499dcc 100644 --- a/packages/worker/src/api/controllers/global/tests/configs.spec.ts +++ b/packages/worker/src/api/controllers/global/tests/configs.spec.ts @@ -13,10 +13,6 @@ describe("Global configs controller", () => { await config.afterAll() }) - afterEach(() => { - jest.resetAllMocks() - }) - it("Should strip secrets when pulling AI config", async () => { const data = structures.configs.ai() await config.api.configs.saveConfig(data) From 5a46e16b8d3f75b9d9de74e43a9865b1cbf86375 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 24 Oct 2024 16:54:08 +0100 Subject: [PATCH 04/38] Adding some tests around the openAPI public APIs to make sure the security works the way we expect, do not redirect API requests. --- packages/backend-core/src/context/identity.ts | 2 +- .../src/middleware/authenticated.ts | 50 +++++--- .../src/api/routes/public/tests/Request.ts | 110 ++++++++++++++++++ .../{metrics.spec.js => metrics.spec.ts} | 2 +- .../api/routes/public/tests/security.spec.ts | 74 ++++++++++++ .../src/api/routes/public/tests/utils.ts | 27 +++-- packages/server/src/middleware/currentapp.ts | 4 +- packages/server/src/middleware/utils.ts | 13 ++- packages/types/package.json | 3 +- packages/types/src/sdk/koa.ts | 8 ++ 10 files changed, 258 insertions(+), 35 deletions(-) create mode 100644 packages/server/src/api/routes/public/tests/Request.ts rename packages/server/src/api/routes/public/tests/{metrics.spec.js => metrics.spec.ts} (94%) create mode 100644 packages/server/src/api/routes/public/tests/security.spec.ts diff --git a/packages/backend-core/src/context/identity.ts b/packages/backend-core/src/context/identity.ts index 84de3b68c9..41f4b1eb66 100644 --- a/packages/backend-core/src/context/identity.ts +++ b/packages/backend-core/src/context/identity.ts @@ -27,7 +27,7 @@ export function doInUserContext(user: User, ctx: Ctx, task: any) { hostInfo: { ipAddress: ctx.request.ip, // filled in by koa-useragent package - userAgent: ctx.userAgent._agent.source, + userAgent: ctx.userAgent.source, }, } return doInIdentityContext(userContext, task) diff --git a/packages/backend-core/src/middleware/authenticated.ts b/packages/backend-core/src/middleware/authenticated.ts index 85aa2293ef..b713f509e0 100644 --- a/packages/backend-core/src/middleware/authenticated.ts +++ b/packages/backend-core/src/middleware/authenticated.ts @@ -1,20 +1,26 @@ import { Cookie, Header } from "../constants" import { - getCookie, clearCookie, - openJwt, + getCookie, isValidInternalAPIKey, + openJwt, } from "../utils" import { getUser } from "../cache/user" import { getSession, updateSessionTTL } from "../security/sessions" import { buildMatcherRegex, matches } from "./matchers" -import { SEPARATOR, queryGlobalView, ViewName } from "../db" -import { getGlobalDB, doInTenant } from "../context" +import { queryGlobalView, SEPARATOR, ViewName } from "../db" +import { doInTenant, getGlobalDB } from "../context" import { decrypt } from "../security/encryption" import * as identity from "../context/identity" import env from "../environment" -import { Ctx, EndpointMatcher, SessionCookie, User } from "@budibase/types" -import { InvalidAPIKeyError, ErrorCode } from "../errors" +import { + Ctx, + EndpointMatcher, + LoginMethod, + SessionCookie, + User, +} from "@budibase/types" +import { ErrorCode, InvalidAPIKeyError } from "../errors" import tracer from "dd-trace" const ONE_MINUTE = env.SESSION_UPDATE_PERIOD @@ -26,16 +32,18 @@ interface FinaliseOpts { internal?: boolean publicEndpoint?: boolean version?: string - user?: any + user?: User | { tenantId: string } + loginMethod?: LoginMethod } function timeMinusOneMinute() { return new Date(Date.now() - ONE_MINUTE).toISOString() } -function finalise(ctx: any, opts: FinaliseOpts = {}) { +function finalise(ctx: Ctx, opts: FinaliseOpts = {}) { ctx.publicEndpoint = opts.publicEndpoint || false ctx.isAuthenticated = opts.authenticated || false + ctx.loginMethod = opts.loginMethod ctx.user = opts.user ctx.internal = opts.internal || false ctx.version = opts.version @@ -120,9 +128,10 @@ export default function ( } const tenantId = ctx.request.headers[Header.TENANT_ID] - let authenticated = false, - user = null, - internal = false + let authenticated: boolean = false, + user: User | { tenantId: string } | undefined = undefined, + internal: boolean = false, + loginMethod: LoginMethod | undefined = undefined if (authCookie && !apiKey) { const sessionId = authCookie.sessionId const userId = authCookie.userId @@ -146,6 +155,7 @@ export default function ( } // @ts-ignore user.csrfToken = session.csrfToken + loginMethod = LoginMethod.COOKIE if (session?.lastAccessedAt < timeMinusOneMinute()) { // make sure we denote that the session is still in use @@ -170,17 +180,16 @@ export default function ( apiKey, populateUser ) - if (valid && foundUser) { + if (valid) { authenticated = true + loginMethod = LoginMethod.API_KEY user = foundUser - } else if (valid) { - authenticated = true - internal = true + internal = !foundUser } } if (!user && tenantId) { user = { tenantId } - } else if (user) { + } else if (user && "password" in user) { delete user.password } // be explicit @@ -204,7 +213,14 @@ export default function ( } // isAuthenticated is a function, so use a variable to be able to check authed state - finalise(ctx, { authenticated, user, internal, version, publicEndpoint }) + finalise(ctx, { + authenticated, + user, + internal, + version, + publicEndpoint, + loginMethod, + }) if (isUser(user)) { return identity.doInUserContext(user, ctx, next) diff --git a/packages/server/src/api/routes/public/tests/Request.ts b/packages/server/src/api/routes/public/tests/Request.ts new file mode 100644 index 0000000000..70e83d711d --- /dev/null +++ b/packages/server/src/api/routes/public/tests/Request.ts @@ -0,0 +1,110 @@ +import { User, Table, SearchFilters, Row } from "@budibase/types" +import { HttpMethod, MakeRequestResponse, generateMakeRequest } from "./utils" +import TestConfiguration from "../../../../tests/utilities/TestConfiguration" +import { Expectations } from "../../../../tests/utilities/api/base" + +type RequestOpts = { internal?: boolean; appId?: string } + +type PublicAPIExpectations = Omit + +export class PublicAPIRequest { + private makeRequest: MakeRequestResponse | undefined + private appId: string | undefined + private _tables: PublicTableAPI | undefined + private _rows: PublicRowAPI | undefined + private _apiKey: string | undefined + + async init(config: TestConfiguration, user: User, opts?: RequestOpts) { + this._apiKey = await config.generateApiKey(user._id) + this.makeRequest = generateMakeRequest(this.apiKey, opts) + this.appId = opts?.appId + this._tables = new PublicTableAPI(this) + this._rows = new PublicRowAPI(this) + return this + } + + opts(opts: RequestOpts) { + if (opts.appId) { + this.appId = opts.appId + } + this.makeRequest = generateMakeRequest(this.apiKey, opts) + } + + async send( + method: HttpMethod, + endpoint: string, + body?: any, + expectations?: PublicAPIExpectations + ) { + if (!this.makeRequest) { + throw new Error("Init has not been called") + } + const res = await this.makeRequest(method, endpoint, body, this.appId) + if (expectations?.status) { + expect(res.status).toEqual(expectations.status) + } + if (expectations?.body) { + expect(res.body).toEqual(expectations?.body) + } + return res.body + } + + get apiKey(): string { + if (!this._apiKey) { + throw new Error("Init has not been called") + } + return this._apiKey + } + + get tables(): PublicTableAPI { + if (!this._tables) { + throw new Error("Init has not been called") + } + return this._tables + } + + get rows(): PublicRowAPI { + if (!this._rows) { + throw new Error("Init has not been called") + } + return this._rows + } +} + +export class PublicTableAPI { + request: PublicAPIRequest + + constructor(request: PublicAPIRequest) { + this.request = request + } + + async create( + table: Table, + expectations?: PublicAPIExpectations + ): Promise<{ data: Table }> { + return this.request.send("post", "/tables", table, expectations) + } +} + +export class PublicRowAPI { + request: PublicAPIRequest + + constructor(request: PublicAPIRequest) { + this.request = request + } + + async search( + tableId: string, + query: SearchFilters, + expectations?: PublicAPIExpectations + ): Promise<{ data: Row[] }> { + return this.request.send( + "post", + `/tables/${tableId}/rows/search`, + { + query, + }, + expectations + ) + } +} diff --git a/packages/server/src/api/routes/public/tests/metrics.spec.js b/packages/server/src/api/routes/public/tests/metrics.spec.ts similarity index 94% rename from packages/server/src/api/routes/public/tests/metrics.spec.js rename to packages/server/src/api/routes/public/tests/metrics.spec.ts index 2fb5e91000..84d47dfa1d 100644 --- a/packages/server/src/api/routes/public/tests/metrics.spec.js +++ b/packages/server/src/api/routes/public/tests/metrics.spec.ts @@ -1,4 +1,4 @@ -const setup = require("../../tests/utilities") +import * as setup from "../../tests/utilities" describe("/metrics", () => { let request = setup.getRequest() diff --git a/packages/server/src/api/routes/public/tests/security.spec.ts b/packages/server/src/api/routes/public/tests/security.spec.ts new file mode 100644 index 0000000000..971cb086eb --- /dev/null +++ b/packages/server/src/api/routes/public/tests/security.spec.ts @@ -0,0 +1,74 @@ +import * as setup from "../../tests/utilities" +import { roles } from "@budibase/backend-core" +import { basicTable } from "../../../../tests/utilities/structures" +import { Table, User } from "@budibase/types" +import { PublicAPIRequest } from "./Request" + +const BROWSER_USER_AGENT = + "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/128.0.0.0 Safari/537.36" + +describe("check public API security", () => { + const config = setup.getConfig() + let builderRequest: PublicAPIRequest, + appUserRequest: PublicAPIRequest, + table: Table, + appUser: User + + beforeAll(async () => { + await config.init() + const builderUser = await config.globalUser() + appUser = await config.globalUser({ + builder: { global: false }, + roles: { + [config.getProdAppId()]: roles.BUILTIN_ROLE_IDS.BASIC, + }, + }) + builderRequest = await new PublicAPIRequest().init(config, builderUser) + appUserRequest = await new PublicAPIRequest().init(config, appUser) + table = (await builderRequest.tables.create(basicTable())).data + }) + + it("should allow with builder API key", async () => { + const res = await builderRequest.rows.search( + table._id!, + {}, + { + status: 200, + } + ) + expect(res.data.length).toEqual(0) + }) + + it("should 403 when from browser, but API key", async () => { + await appUserRequest.rows.search( + table._id!, + {}, + { + status: 403, + } + ) + }) + + it("should re-direct when using cookie", async () => { + const headers = await config.login({ + userId: appUser._id!, + builder: false, + prodApp: false, + }) + await config.withHeaders( + { + ...headers, + "User-Agent": BROWSER_USER_AGENT, + }, + async () => { + await config.api.row.search( + table._id!, + { query: {} }, + { + status: 302, + } + ) + } + ) + }) +}) diff --git a/packages/server/src/api/routes/public/tests/utils.ts b/packages/server/src/api/routes/public/tests/utils.ts index 1b57682af9..4fb048a540 100644 --- a/packages/server/src/api/routes/public/tests/utils.ts +++ b/packages/server/src/api/routes/public/tests/utils.ts @@ -21,17 +21,19 @@ export type MakeRequestWithFormDataResponse = ( function base( apiKey: string, endpoint: string, - intAppId: string | null, - isInternal: boolean + opts?: { + intAppId?: string + internal?: boolean + } ) { const extraHeaders: any = { "x-budibase-api-key": apiKey, } - if (intAppId) { - extraHeaders["x-budibase-app-id"] = intAppId + if (opts?.intAppId) { + extraHeaders["x-budibase-app-id"] = opts.intAppId } - const url = isInternal + const url = opts?.internal ? endpoint : checkSlashesInUrl(`/api/public/v1/${endpoint}`) return { headers: extraHeaders, url } @@ -39,7 +41,7 @@ function base( export function generateMakeRequest( apiKey: string, - isInternal = false + opts?: { internal?: boolean } ): MakeRequestResponse { const request = setup.getRequest()! const config = setup.getConfig()! @@ -47,9 +49,12 @@ export function generateMakeRequest( method: HttpMethod, endpoint: string, body?: any, - intAppId: string | null = config.getAppId() + intAppId: string | undefined = config.getAppId() ) => { - const { headers, url } = base(apiKey, endpoint, intAppId, isInternal) + const { headers, url } = base(apiKey, endpoint, { ...opts, intAppId }) + if (body && typeof body !== "string") { + headers["Content-Type"] = "application/json" + } const req = request[method](url).set(config.defaultHeaders(headers)) if (body) { req.send(body) @@ -62,7 +67,7 @@ export function generateMakeRequest( export function generateMakeRequestWithFormData( apiKey: string, - isInternal = false + opts?: { internal?: boolean; browser?: boolean } ): MakeRequestWithFormDataResponse { const request = setup.getRequest()! const config = setup.getConfig()! @@ -70,9 +75,9 @@ export function generateMakeRequestWithFormData( method: HttpMethod, endpoint: string, fields: Record, - intAppId: string | null = config.getAppId() + intAppId: string | undefined = config.getAppId() ) => { - const { headers, url } = base(apiKey, endpoint, intAppId, isInternal) + const { headers, url } = base(apiKey, endpoint, { ...opts, intAppId }) const req = request[method](url).set(config.defaultHeaders(headers)) for (let [field, value] of Object.entries(fields)) { if (typeof value === "string") { diff --git a/packages/server/src/middleware/currentapp.ts b/packages/server/src/middleware/currentapp.ts index d616377298..d5840712a0 100644 --- a/packages/server/src/middleware/currentapp.ts +++ b/packages/server/src/middleware/currentapp.ts @@ -10,7 +10,7 @@ import { import { generateUserMetadataID, isDevAppID } from "../db/utils" import { getCachedSelf } from "../utilities/global" import env from "../environment" -import { isWebhookEndpoint } from "./utils" +import { isWebhookEndpoint, isBrowser, isApiKey } from "./utils" import { UserCtx, ContextUser } from "@budibase/types" import tracer from "dd-trace" @@ -27,7 +27,7 @@ export default async (ctx: UserCtx, next: any) => { } // deny access to application preview - if (!env.isTest()) { + if (isBrowser(ctx) && !isApiKey(ctx)) { if ( isDevAppID(requestAppId) && !isWebhookEndpoint(ctx) && diff --git a/packages/server/src/middleware/utils.ts b/packages/server/src/middleware/utils.ts index 714df12f38..650260b136 100644 --- a/packages/server/src/middleware/utils.ts +++ b/packages/server/src/middleware/utils.ts @@ -1,9 +1,18 @@ -import { BBContext } from "@budibase/types" +import { LoginMethod, UserCtx } from "@budibase/types" const WEBHOOK_ENDPOINTS = new RegExp( ["webhooks/trigger", "webhooks/schema"].join("|") ) -export function isWebhookEndpoint(ctx: BBContext) { +export function isWebhookEndpoint(ctx: UserCtx) { return WEBHOOK_ENDPOINTS.test(ctx.request.url) } + +export function isBrowser(ctx: UserCtx) { + const browser = ctx.userAgent.browser + return browser !== "unknown" +} + +export function isApiKey(ctx: UserCtx) { + return ctx.loginMethod === LoginMethod.API_KEY +} diff --git a/packages/types/package.json b/packages/types/package.json index bce25c7086..d132383081 100644 --- a/packages/types/package.json +++ b/packages/types/package.json @@ -19,7 +19,8 @@ "@types/koa": "2.13.4", "@types/redlock": "4.0.7", "rimraf": "3.0.2", - "typescript": "5.5.2" + "typescript": "5.5.2", + "koa-useragent": "^4.1.0" }, "dependencies": { "scim-patch": "^0.8.1" diff --git a/packages/types/src/sdk/koa.ts b/packages/types/src/sdk/koa.ts index a7df701171..38622b974a 100644 --- a/packages/types/src/sdk/koa.ts +++ b/packages/types/src/sdk/koa.ts @@ -2,6 +2,12 @@ import { Context, Request } from "koa" import { User, Role, UserRoles, Account, ConfigType } from "../documents" import { FeatureFlag, License } from "../sdk" import { Files } from "formidable" +import { UserAgentContext } from "koa-useragent" + +export enum LoginMethod { + API_KEY = "api_key", + COOKIE = "cookie", +} export interface ContextUser extends Omit { globalId?: string @@ -31,6 +37,7 @@ export interface BBRequest extends Request { export interface Ctx extends Context { request: BBRequest body: ResponseBody + userAgent: UserAgentContext["userAgent"] } /** @@ -40,6 +47,7 @@ export interface UserCtx extends Ctx { user: ContextUser roleId?: string + loginMethod?: LoginMethod } /** From 0863a1167cba60bfd962840e0f0da22eaa1d3173 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 24 Oct 2024 17:41:34 +0100 Subject: [PATCH 05/38] Updating OpenAPI definition to contain all required variables. --- packages/server/specs/generate.ts | 18 +++++-------- packages/server/specs/openapi.json | 30 ++++++++++++---------- packages/server/specs/openapi.yaml | 22 +++++++++------- packages/server/src/definitions/openapi.ts | 21 ++++++++++----- 4 files changed, 51 insertions(+), 40 deletions(-) diff --git a/packages/server/specs/generate.ts b/packages/server/specs/generate.ts index eea00f83aa..8f6376195f 100644 --- a/packages/server/specs/generate.ts +++ b/packages/server/specs/generate.ts @@ -20,19 +20,15 @@ const options = { { url: "https://budibase.app/api/public/v1", description: "Budibase Cloud API", - }, - { - url: "{protocol}://{hostname}/api/public/v1", - description: "Budibase self hosted API", variables: { - protocol: { - default: "http", - description: - "Whether HTTP or HTTPS should be used to communicate with your Budibase instance.", + apiKey: { + default: "", + description: "The API key of the user to assume for API call.", }, - hostname: { - default: "localhost:10000", - description: "The URL of your Budibase instance.", + appId: { + default: "", + description: + "The ID of the app the calls will be executed within the context of, this should start with app_ (production) or app_dev (development).", }, }, }, diff --git a/packages/server/specs/openapi.json b/packages/server/specs/openapi.json index b21554505b..c96296a8bb 100644 --- a/packages/server/specs/openapi.json +++ b/packages/server/specs/openapi.json @@ -8,19 +8,15 @@ "servers": [ { "url": "https://budibase.app/api/public/v1", - "description": "Budibase Cloud API" - }, - { - "url": "{protocol}://{hostname}/api/public/v1", - "description": "Budibase self hosted API", + "description": "Budibase Cloud API", "variables": { - "protocol": { - "default": "http", - "description": "Whether HTTP or HTTPS should be used to communicate with your Budibase instance." + "apiKey": { + "default": "", + "description": "The API key of the user to assume for API call." }, - "hostname": { - "default": "localhost:10000", - "description": "The URL of your Budibase instance." + "appId": { + "default": "", + "description": "The ID of the app the calls will be executed within the context of, this should start with app_ (production) or app_dev (development)." } } } @@ -833,7 +829,8 @@ "type": "string", "enum": [ "static", - "dynamic" + "dynamic", + "ai" ], "description": "Defines whether this is a static or dynamic formula." } @@ -857,6 +854,7 @@ "link", "formula", "auto", + "ai", "json", "internal", "barcodeqr", @@ -1042,7 +1040,8 @@ "type": "string", "enum": [ "static", - "dynamic" + "dynamic", + "ai" ], "description": "Defines whether this is a static or dynamic formula." } @@ -1066,6 +1065,7 @@ "link", "formula", "auto", + "ai", "json", "internal", "barcodeqr", @@ -1262,7 +1262,8 @@ "type": "string", "enum": [ "static", - "dynamic" + "dynamic", + "ai" ], "description": "Defines whether this is a static or dynamic formula." } @@ -1286,6 +1287,7 @@ "link", "formula", "auto", + "ai", "json", "internal", "barcodeqr", diff --git a/packages/server/specs/openapi.yaml b/packages/server/specs/openapi.yaml index 6a2ae89c61..9ac7b7533a 100644 --- a/packages/server/specs/openapi.yaml +++ b/packages/server/specs/openapi.yaml @@ -6,16 +6,14 @@ info: servers: - url: https://budibase.app/api/public/v1 description: Budibase Cloud API - - url: "{protocol}://{hostname}/api/public/v1" - description: Budibase self hosted API variables: - protocol: - default: http - description: Whether HTTP or HTTPS should be used to communicate with your - Budibase instance. - hostname: - default: localhost:10000 - description: The URL of your Budibase instance. + apiKey: + default: + description: The API key of the user to assume for API call. + appId: + default: + description: The ID of the app the calls will be executed within the context of, + this should start with app_ (production) or app_dev (development). components: parameters: tableId: @@ -761,6 +759,7 @@ components: enum: - static - dynamic + - ai description: Defines whether this is a static or dynamic formula. - type: object properties: @@ -779,6 +778,7 @@ components: - link - formula - auto + - ai - json - internal - barcodeqr @@ -929,6 +929,7 @@ components: enum: - static - dynamic + - ai description: Defines whether this is a static or dynamic formula. - type: object properties: @@ -947,6 +948,7 @@ components: - link - formula - auto + - ai - json - internal - barcodeqr @@ -1104,6 +1106,7 @@ components: enum: - static - dynamic + - ai description: Defines whether this is a static or dynamic formula. - type: object properties: @@ -1122,6 +1125,7 @@ components: - link - formula - auto + - ai - json - internal - barcodeqr diff --git a/packages/server/src/definitions/openapi.ts b/packages/server/src/definitions/openapi.ts index cc3c8a7d4d..3abf7e6fa7 100644 --- a/packages/server/src/definitions/openapi.ts +++ b/packages/server/src/definitions/openapi.ts @@ -257,7 +257,7 @@ export interface components { * @description Defines whether this is a static or dynamic formula. * @enum {string} */ - formulaType?: "static" | "dynamic"; + formulaType?: "static" | "dynamic" | "ai"; } | { /** @@ -277,11 +277,14 @@ export interface components { | "link" | "formula" | "auto" + | "ai" | "json" | "internal" | "barcodeqr" + | "signature_single" | "bigint" - | "bb_reference"; + | "bb_reference" + | "bb_reference_single"; /** @description A constraint can be applied to the column which will be validated against when a row is saved. */ constraints?: { /** @enum {string} */ @@ -366,7 +369,7 @@ export interface components { * @description Defines whether this is a static or dynamic formula. * @enum {string} */ - formulaType?: "static" | "dynamic"; + formulaType?: "static" | "dynamic" | "ai"; } | { /** @@ -386,11 +389,14 @@ export interface components { | "link" | "formula" | "auto" + | "ai" | "json" | "internal" | "barcodeqr" + | "signature_single" | "bigint" - | "bb_reference"; + | "bb_reference" + | "bb_reference_single"; /** @description A constraint can be applied to the column which will be validated against when a row is saved. */ constraints?: { /** @enum {string} */ @@ -477,7 +483,7 @@ export interface components { * @description Defines whether this is a static or dynamic formula. * @enum {string} */ - formulaType?: "static" | "dynamic"; + formulaType?: "static" | "dynamic" | "ai"; } | { /** @@ -497,11 +503,14 @@ export interface components { | "link" | "formula" | "auto" + | "ai" | "json" | "internal" | "barcodeqr" + | "signature_single" | "bigint" - | "bb_reference"; + | "bb_reference" + | "bb_reference_single"; /** @description A constraint can be applied to the column which will be validated against when a row is saved. */ constraints?: { /** @enum {string} */ From 68354cc50f80576b6ae94a8dc8fb72336d791816 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 24 Oct 2024 17:48:25 +0100 Subject: [PATCH 06/38] Defaulting app ID to variable. --- packages/server/specs/openapi.json | 2 ++ packages/server/specs/openapi.yaml | 2 ++ packages/server/specs/parameters.ts | 2 ++ 3 files changed, 6 insertions(+) diff --git a/packages/server/specs/openapi.json b/packages/server/specs/openapi.json index c96296a8bb..f3091a1fc7 100644 --- a/packages/server/specs/openapi.json +++ b/packages/server/specs/openapi.json @@ -47,6 +47,7 @@ "required": true, "description": "The ID of the app which this request is targeting.", "schema": { + "default": "{{ appId }}", "type": "string" } }, @@ -56,6 +57,7 @@ "required": true, "description": "The ID of the app which this request is targeting.", "schema": { + "default": "{{ appId }}", "type": "string" } }, diff --git a/packages/server/specs/openapi.yaml b/packages/server/specs/openapi.yaml index 9ac7b7533a..1e9b9921cf 100644 --- a/packages/server/specs/openapi.yaml +++ b/packages/server/specs/openapi.yaml @@ -36,6 +36,7 @@ components: required: true description: The ID of the app which this request is targeting. schema: + default: "{{ appId }}" type: string appIdUrl: in: path @@ -43,6 +44,7 @@ components: required: true description: The ID of the app which this request is targeting. schema: + default: "{{ appId }}" type: string queryId: in: path diff --git a/packages/server/specs/parameters.ts b/packages/server/specs/parameters.ts index a928026bb1..a4f2b27ae4 100644 --- a/packages/server/specs/parameters.ts +++ b/packages/server/specs/parameters.ts @@ -24,6 +24,7 @@ export const appId = { required: true, description: "The ID of the app which this request is targeting.", schema: { + default: "{{ appId }}", type: "string", }, } @@ -34,6 +35,7 @@ export const appIdUrl = { required: true, description: "The ID of the app which this request is targeting.", schema: { + default: "{{ appId }}", type: "string", }, } From c33f3319041a6662364c51bae297ba9d907f474c Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 24 Oct 2024 18:08:49 +0100 Subject: [PATCH 07/38] Test fix. --- packages/server/src/middleware/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/middleware/utils.ts b/packages/server/src/middleware/utils.ts index 650260b136..8e7c623e2a 100644 --- a/packages/server/src/middleware/utils.ts +++ b/packages/server/src/middleware/utils.ts @@ -9,8 +9,8 @@ export function isWebhookEndpoint(ctx: UserCtx) { } export function isBrowser(ctx: UserCtx) { - const browser = ctx.userAgent.browser - return browser !== "unknown" + const browser = ctx.userAgent?.browser + return browser && browser !== "unknown" } export function isApiKey(ctx: UserCtx) { From f1fa0a3a6f2963accc49c28442895c8a004711e1 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 25 Oct 2024 10:41:20 +0100 Subject: [PATCH 08/38] Fixing tests, updating to typescript. --- .../api/routes/public/tests/security.spec.ts | 5 +- .../{routing.spec.js => routing.spec.ts} | 61 ++++++++++++------- ...{currentapp.spec.js => currentapp.spec.ts} | 16 +++-- .../src/tests/utilities/TestConfiguration.ts | 5 ++ 4 files changed, 55 insertions(+), 32 deletions(-) rename packages/server/src/api/routes/tests/{routing.spec.js => routing.spec.ts} (72%) rename packages/server/src/middleware/tests/{currentapp.spec.js => currentapp.spec.ts} (94%) diff --git a/packages/server/src/api/routes/public/tests/security.spec.ts b/packages/server/src/api/routes/public/tests/security.spec.ts index 971cb086eb..a285bc7736 100644 --- a/packages/server/src/api/routes/public/tests/security.spec.ts +++ b/packages/server/src/api/routes/public/tests/security.spec.ts @@ -4,9 +4,6 @@ import { basicTable } from "../../../../tests/utilities/structures" import { Table, User } from "@budibase/types" import { PublicAPIRequest } from "./Request" -const BROWSER_USER_AGENT = - "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/128.0.0.0 Safari/537.36" - describe("check public API security", () => { const config = setup.getConfig() let builderRequest: PublicAPIRequest, @@ -58,7 +55,7 @@ describe("check public API security", () => { await config.withHeaders( { ...headers, - "User-Agent": BROWSER_USER_AGENT, + "User-Agent": config.browserUserAgent(), }, async () => { await config.api.row.search( diff --git a/packages/server/src/api/routes/tests/routing.spec.js b/packages/server/src/api/routes/tests/routing.spec.ts similarity index 72% rename from packages/server/src/api/routes/tests/routing.spec.js rename to packages/server/src/api/routes/tests/routing.spec.ts index 5bdd4ee852..0d4ffc7c3e 100644 --- a/packages/server/src/api/routes/tests/routing.spec.js +++ b/packages/server/src/api/routes/tests/routing.spec.ts @@ -1,9 +1,10 @@ -const setup = require("./utilities") -const { basicScreen, powerScreen } = setup.structures -const { checkBuilderEndpoint, runInProd } = require("./utilities/TestFunctions") -const { roles } = require("@budibase/backend-core") -const { BUILTIN_ROLE_IDS } = roles +import * as setup from "./utilities" +import { checkBuilderEndpoint, runInProd } from "./utilities/TestFunctions" +import { roles } from "@budibase/backend-core" +import { Screen } from "@budibase/types" +const { BUILTIN_ROLE_IDS } = roles +const { basicScreen, powerScreen } = setup.structures const route = "/test" // there are checks which are disabled in test env, @@ -12,7 +13,7 @@ const route = "/test" describe("/routing", () => { let request = setup.getRequest() let config = setup.getConfig() - let basic, power + let basic: Screen, power: Screen afterAll(setup.afterAll) @@ -25,26 +26,40 @@ describe("/routing", () => { describe("fetch", () => { it("prevents a public user from accessing development app", async () => { - await runInProd(() => { - return request - .get(`/api/routing/client`) - .set(config.publicHeaders({ prodApp: false })) - .expect(302) - }) + await config.withHeaders( + { + "User-Agent": config.browserUserAgent(), + }, + async () => { + await runInProd(() => { + return request + .get(`/api/routing/client`) + .set(config.publicHeaders({ prodApp: false })) + .expect(302) + }) + } + ) }) it("prevents a non builder from accessing development app", async () => { - await runInProd(async () => { - return request - .get(`/api/routing/client`) - .set( - await config.roleHeaders({ - roleId: BUILTIN_ROLE_IDS.BASIC, - prodApp: false, - }) - ) - .expect(302) - }) + await config.withHeaders( + { + "User-Agent": config.browserUserAgent(), + }, + async () => { + await runInProd(async () => { + return request + .get(`/api/routing/client`) + .set( + await config.roleHeaders({ + roleId: BUILTIN_ROLE_IDS.BASIC, + prodApp: false, + }) + ) + .expect(302) + }) + } + ) }) it("returns the correct routing for basic user", async () => { const res = await request diff --git a/packages/server/src/middleware/tests/currentapp.spec.js b/packages/server/src/middleware/tests/currentapp.spec.ts similarity index 94% rename from packages/server/src/middleware/tests/currentapp.spec.js rename to packages/server/src/middleware/tests/currentapp.spec.ts index 22e47b0a6e..202d9f96d5 100644 --- a/packages/server/src/middleware/tests/currentapp.spec.js +++ b/packages/server/src/middleware/tests/currentapp.spec.ts @@ -1,4 +1,6 @@ -require("../../db").init() +import * as db from "../../db" + +db.init() mockAuthWithNoCookie() mockWorker() mockUserGroups() @@ -45,7 +47,7 @@ function mockAuthWithNoCookie() { }, cache: { user: { - getUser: async id => { + getUser: async () => { return { _id: "us_uuid1", } @@ -82,7 +84,7 @@ function mockAuthWithCookie() { }, cache: { user: { - getUser: async id => { + getUser: async () => { return { _id: "us_uuid1", } @@ -94,6 +96,10 @@ function mockAuthWithCookie() { } class TestConfiguration { + next: jest.MockedFunction + throw: jest.MockedFunction + ctx: any + constructor() { this.next = jest.fn() this.throw = jest.fn() @@ -130,7 +136,7 @@ class TestConfiguration { } describe("Current app middleware", () => { - let config + let config: TestConfiguration beforeEach(() => { config = new TestConfiguration() @@ -192,7 +198,7 @@ describe("Current app middleware", () => { }, cache: { user: { - getUser: async id => { + getUser: async () => { return { _id: "us_uuid1", } diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 713f8b31de..5ed60a59b6 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -423,6 +423,7 @@ export default class TestConfiguration { Accept: "application/json", Cookie: [`${constants.Cookie.Auth}=${authToken}`], [constants.Header.APP_ID]: appId, + ...this.temporaryHeaders, } }) } @@ -527,6 +528,10 @@ export default class TestConfiguration { return this.login({ userId: email, roleId, builder, prodApp }) } + browserUserAgent() { + return "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/128.0.0.0 Safari/537.36" + } + // TENANCY tenantHost() { From 5e4cfafa9620b83c7f1b3bb33b8655d497a9a044 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 25 Oct 2024 11:53:27 +0100 Subject: [PATCH 09/38] Expose bigints as a column to do calculations on. --- .../buttons/grid/GridViewCalculationButton.svelte | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte b/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte index 72216f3b3b..db58ba4a8f 100644 --- a/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte +++ b/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte @@ -7,7 +7,12 @@ Icon, Multiselect, } from "@budibase/bbui" - import { CalculationType, canGroupBy, FieldType } from "@budibase/types" + import { + CalculationType, + canGroupBy, + FieldType, + isNumeric, + } from "@budibase/types" import InfoDisplay from "pages/builder/app/[application]/design/[screenId]/[componentId]/_components/Component/InfoDisplay.svelte" import { getContext } from "svelte" @@ -90,10 +95,7 @@ return Object.entries(schema) .filter(([field, fieldSchema]) => { // Only allow numeric fields that are not calculations themselves - if ( - fieldSchema.calculationType || - fieldSchema.type !== FieldType.NUMBER - ) { + if (fieldSchema.calculationType || !isNumeric(fieldSchema.type)) { return false } // Don't allow duplicates From e53ec31305741a9d1b8201296cab25ab0d1f49c1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 25 Oct 2024 13:20:15 +0200 Subject: [PATCH 10/38] Add row create test --- .../server/src/api/routes/tests/row.spec.ts | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 31067ab40f..d79a8a7379 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -939,6 +939,74 @@ describe.each([ }) }) }) + + describe("relations to same table", () => { + let relatedRows: Row[] + + beforeAll(async () => { + const relatedTable = await config.api.table.save( + defaultTable({ + schema: { + name: { name: "name", type: FieldType.STRING }, + }, + }) + ) + const relatedTableId = relatedTable._id! + table = await config.api.table.save( + defaultTable({ + schema: { + name: { name: "name", type: FieldType.STRING }, + related1: { + type: FieldType.LINK, + name: "related1", + fieldName: "main1", + tableId: relatedTableId, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + related2: { + type: FieldType.LINK, + name: "related2", + fieldName: "main2", + tableId: relatedTableId, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + }, + }) + ) + relatedRows = await Promise.all([ + config.api.row.save(relatedTableId, { name: "foo" }), + config.api.row.save(relatedTableId, { name: "bar" }), + config.api.row.save(relatedTableId, { name: "baz" }), + config.api.row.save(relatedTableId, { name: "boo" }), + ]) + }) + + it("can create rows with both relationships", async () => { + const row = await config.api.row.save(table._id!, { + name: "test", + related1: [relatedRows[0]._id!], + related2: [relatedRows[1]._id!], + }) + + expect(row).toEqual( + expect.objectContaining({ + name: "test", + related1: [ + { + _id: relatedRows[0]._id, + primaryDisplay: relatedRows[0].name, + }, + ], + related2: [ + { + _id: relatedRows[1]._id, + primaryDisplay: relatedRows[1].name, + }, + ], + }) + ) + }) + }) }) describe("get", () => { From d044d56801fc0f53184388da21ecee9f88e2c124 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 25 Oct 2024 13:25:28 +0200 Subject: [PATCH 11/38] Add more tests --- .../server/src/api/routes/tests/row.spec.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index d79a8a7379..4bfd612775 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -1006,6 +1006,36 @@ describe.each([ }) ) }) + + it("can create rows with no relationships", async () => { + const row = await config.api.row.save(table._id!, { + name: "test", + }) + + expect(row.related1).toBeUndefined() + expect(row.related2).toBeUndefined() + }) + + it("can create rows with only one relationships field", async () => { + const row = await config.api.row.save(table._id!, { + name: "test", + related1: [], + related2: [relatedRows[1]._id!], + }) + + expect(row).toEqual( + expect.objectContaining({ + name: "test", + related2: [ + { + _id: relatedRows[1]._id, + primaryDisplay: relatedRows[1].name, + }, + ], + }) + ) + expect(row.related1).toBeUndefined() + }) }) }) From 99c3395cb9d2c970e2db4c22b7eddc35b60583c9 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 25 Oct 2024 13:13:10 +0100 Subject: [PATCH 12/38] Some fixes to Public API test request as per PR comments, as well as removing AI formula type from open API spec (this is used internally, not supposed to be exposed). --- packages/server/specs/openapi.json | 9 +-- packages/server/specs/openapi.yaml | 3 - packages/server/specs/resources/table.ts | 2 +- .../src/api/routes/public/tests/Request.ts | 58 ++++++++----------- .../api/routes/public/tests/security.spec.ts | 4 +- packages/server/src/definitions/openapi.ts | 6 +- 6 files changed, 34 insertions(+), 48 deletions(-) diff --git a/packages/server/specs/openapi.json b/packages/server/specs/openapi.json index f3091a1fc7..685e168c66 100644 --- a/packages/server/specs/openapi.json +++ b/packages/server/specs/openapi.json @@ -831,8 +831,7 @@ "type": "string", "enum": [ "static", - "dynamic", - "ai" + "dynamic" ], "description": "Defines whether this is a static or dynamic formula." } @@ -1042,8 +1041,7 @@ "type": "string", "enum": [ "static", - "dynamic", - "ai" + "dynamic" ], "description": "Defines whether this is a static or dynamic formula." } @@ -1264,8 +1262,7 @@ "type": "string", "enum": [ "static", - "dynamic", - "ai" + "dynamic" ], "description": "Defines whether this is a static or dynamic formula." } diff --git a/packages/server/specs/openapi.yaml b/packages/server/specs/openapi.yaml index 1e9b9921cf..61a5d0b57f 100644 --- a/packages/server/specs/openapi.yaml +++ b/packages/server/specs/openapi.yaml @@ -761,7 +761,6 @@ components: enum: - static - dynamic - - ai description: Defines whether this is a static or dynamic formula. - type: object properties: @@ -931,7 +930,6 @@ components: enum: - static - dynamic - - ai description: Defines whether this is a static or dynamic formula. - type: object properties: @@ -1108,7 +1106,6 @@ components: enum: - static - dynamic - - ai description: Defines whether this is a static or dynamic formula. - type: object properties: diff --git a/packages/server/specs/resources/table.ts b/packages/server/specs/resources/table.ts index 7f90769b07..c068dbd48b 100644 --- a/packages/server/specs/resources/table.ts +++ b/packages/server/specs/resources/table.ts @@ -138,7 +138,7 @@ const tableSchema = { }, formulaType: { type: "string", - enum: Object.values(FormulaType), + enum: [FormulaType.STATIC, FormulaType.DYNAMIC], description: "Defines whether this is a static or dynamic formula.", }, diff --git a/packages/server/src/api/routes/public/tests/Request.ts b/packages/server/src/api/routes/public/tests/Request.ts index 70e83d711d..92a4996124 100644 --- a/packages/server/src/api/routes/public/tests/Request.ts +++ b/packages/server/src/api/routes/public/tests/Request.ts @@ -5,22 +5,35 @@ import { Expectations } from "../../../../tests/utilities/api/base" type RequestOpts = { internal?: boolean; appId?: string } -type PublicAPIExpectations = Omit +export interface PublicAPIExpectations { + status?: number + body?: Record +} export class PublicAPIRequest { - private makeRequest: MakeRequestResponse | undefined + private makeRequest: MakeRequestResponse private appId: string | undefined - private _tables: PublicTableAPI | undefined - private _rows: PublicRowAPI | undefined - private _apiKey: string | undefined - async init(config: TestConfiguration, user: User, opts?: RequestOpts) { - this._apiKey = await config.generateApiKey(user._id) - this.makeRequest = generateMakeRequest(this.apiKey, opts) - this.appId = opts?.appId - this._tables = new PublicTableAPI(this) - this._rows = new PublicRowAPI(this) - return this + tables: PublicTableAPI + rows: PublicRowAPI + apiKey: string + + private constructor( + apiKey: string, + makeRequest: MakeRequestResponse, + appId?: string + ) { + this.apiKey = apiKey + this.makeRequest = makeRequest + this.appId = appId + this.tables = new PublicTableAPI(this) + this.rows = new PublicRowAPI(this) + } + + static async init(config: TestConfiguration, user: User, opts?: RequestOpts) { + const apiKey = await config.generateApiKey(user._id) + const makeRequest = generateMakeRequest(apiKey, opts) + return new this(apiKey, makeRequest, opts?.appId) } opts(opts: RequestOpts) { @@ -48,27 +61,6 @@ export class PublicAPIRequest { } return res.body } - - get apiKey(): string { - if (!this._apiKey) { - throw new Error("Init has not been called") - } - return this._apiKey - } - - get tables(): PublicTableAPI { - if (!this._tables) { - throw new Error("Init has not been called") - } - return this._tables - } - - get rows(): PublicRowAPI { - if (!this._rows) { - throw new Error("Init has not been called") - } - return this._rows - } } export class PublicTableAPI { diff --git a/packages/server/src/api/routes/public/tests/security.spec.ts b/packages/server/src/api/routes/public/tests/security.spec.ts index a285bc7736..21b4d66843 100644 --- a/packages/server/src/api/routes/public/tests/security.spec.ts +++ b/packages/server/src/api/routes/public/tests/security.spec.ts @@ -20,8 +20,8 @@ describe("check public API security", () => { [config.getProdAppId()]: roles.BUILTIN_ROLE_IDS.BASIC, }, }) - builderRequest = await new PublicAPIRequest().init(config, builderUser) - appUserRequest = await new PublicAPIRequest().init(config, appUser) + builderRequest = await PublicAPIRequest.init(config, builderUser) + appUserRequest = await PublicAPIRequest.init(config, appUser) table = (await builderRequest.tables.create(basicTable())).data }) diff --git a/packages/server/src/definitions/openapi.ts b/packages/server/src/definitions/openapi.ts index 3abf7e6fa7..4479c89d9d 100644 --- a/packages/server/src/definitions/openapi.ts +++ b/packages/server/src/definitions/openapi.ts @@ -257,7 +257,7 @@ export interface components { * @description Defines whether this is a static or dynamic formula. * @enum {string} */ - formulaType?: "static" | "dynamic" | "ai"; + formulaType?: "static" | "dynamic"; } | { /** @@ -369,7 +369,7 @@ export interface components { * @description Defines whether this is a static or dynamic formula. * @enum {string} */ - formulaType?: "static" | "dynamic" | "ai"; + formulaType?: "static" | "dynamic"; } | { /** @@ -483,7 +483,7 @@ export interface components { * @description Defines whether this is a static or dynamic formula. * @enum {string} */ - formulaType?: "static" | "dynamic" | "ai"; + formulaType?: "static" | "dynamic"; } | { /** From 8f17d7f1bf48df724b21034a88782342b615c9c5 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 25 Oct 2024 13:39:01 +0100 Subject: [PATCH 13/38] Updating pro submodule again. --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 6041196305..f6aebba944 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 60411963053b98e0415a1e9285b721fc26c628c8 +Subproject commit f6aebba94451ce47bba551926e5ad72bd75f71c6 From 5efcf97c0d6815d8cc80fe06216b8c5d160602b3 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 25 Oct 2024 15:16:54 +0200 Subject: [PATCH 14/38] Add more tests --- .../server/src/api/routes/tests/row.spec.ts | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 4bfd612775..32788d2783 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -1139,6 +1139,133 @@ describe.each([ const rows = await config.api.row.fetch(table._id!) expect(rows).toHaveLength(1) }) + + describe("relations to same table", () => { + let relatedRows: Row[] + + beforeAll(async () => { + const relatedTable = await config.api.table.save( + defaultTable({ + schema: { + name: { name: "name", type: FieldType.STRING }, + }, + }) + ) + const relatedTableId = relatedTable._id! + table = await config.api.table.save( + defaultTable({ + schema: { + name: { name: "name", type: FieldType.STRING }, + related1: { + type: FieldType.LINK, + name: "related1", + fieldName: "main1", + tableId: relatedTableId, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + related2: { + type: FieldType.LINK, + name: "related2", + fieldName: "main2", + tableId: relatedTableId, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + }, + }) + ) + relatedRows = await Promise.all([ + config.api.row.save(relatedTableId, { name: "foo" }), + config.api.row.save(relatedTableId, { name: "bar" }), + config.api.row.save(relatedTableId, { name: "baz" }), + config.api.row.save(relatedTableId, { name: "boo" }), + ]) + }) + + it("can edit rows with both relationships", async () => { + let row = await config.api.row.save(table._id!, { + name: "test", + related1: [relatedRows[0]._id!], + related2: [relatedRows[1]._id!], + }) + + row = await config.api.row.save(table._id!, { + ...row, + related1: [relatedRows[0]._id!, relatedRows[1]._id!], + related2: [relatedRows[2]._id!], + }) + + expect(row).toEqual( + expect.objectContaining({ + name: "test", + related1: [ + { + _id: relatedRows[1]._id, + primaryDisplay: relatedRows[1].name, + }, + { + _id: relatedRows[0]._id, + primaryDisplay: relatedRows[0].name, + }, + ], + related2: [ + { + _id: relatedRows[2]._id, + primaryDisplay: relatedRows[2].name, + }, + ], + }) + ) + }) + + it("can drop existing relationship", async () => { + let row = await config.api.row.save(table._id!, { + name: "test", + related1: [relatedRows[0]._id!], + related2: [relatedRows[1]._id!], + }) + + row = await config.api.row.save(table._id!, { + ...row, + related1: [], + related2: [relatedRows[2]._id!], + }) + + expect(row).toEqual( + expect.objectContaining({ + name: "test", + related2: [ + { + _id: relatedRows[2]._id, + primaryDisplay: relatedRows[2].name, + }, + ], + }) + ) + expect(row.related1).toBeUndefined() + }) + + it("can drop both relationships", async () => { + let row = await config.api.row.save(table._id!, { + name: "test", + related1: [relatedRows[0]._id!], + related2: [relatedRows[1]._id!], + }) + + row = await config.api.row.save(table._id!, { + ...row, + related1: [], + related2: [], + }) + + expect(row).toEqual( + expect.objectContaining({ + name: "test", + }) + ) + expect(row.related1).toBeUndefined() + expect(row.related2).toBeUndefined() + }) + }) }) describe("patch", () => { From 96be44781779b981de7b3350ff21004e449e3e36 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 25 Oct 2024 15:22:57 +0200 Subject: [PATCH 15/38] Fix many to many --- .../api/controllers/row/ExternalRequest.ts | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index 5ceed002de..527bc4b3ef 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -425,8 +425,12 @@ export class ExternalRequest { */ async lookupRelations(tableId: string, row: Row) { const related: { - [key: string]: { rows: Row[]; isMany: boolean; tableId: string } - } = {} + rows: Row[] + isMany: boolean + tableId: string + field: string + }[] = [] + const { tableName } = breakExternalTableId(tableId) const table = this.tables[tableName] // @ts-ignore @@ -489,11 +493,13 @@ export class ExternalRequest { const storeTo = isManyToMany(field) ? field.throughFrom || linkPrimaryKey : fieldName - related[storeTo] = { + + related.push({ rows, isMany: isManyToMany(field), tableId: relatedTableId, - } + field: storeTo, + }) } return related } @@ -528,7 +534,9 @@ export class ExternalRequest { const linkSecondary = relationshipPrimary[1] - const rows = related[key]?.rows || [] + const rows = + related.find(r => r.tableId === relationship.tableId && r.field === key) + ?.rows || [] const relationshipMatchPredicate = ({ row, @@ -573,12 +581,12 @@ export class ExternalRequest { } } // finally cleanup anything that needs to be removed - for (let [colName, { isMany, rows, tableId }] of Object.entries(related)) { + for (let { isMany, rows, tableId, field } of related) { const table: Table | undefined = this.getTable(tableId) // if it's not the foreign key skip it, nothing to do if ( !table || - (!isMany && table.primary && table.primary.indexOf(colName) !== -1) + (!isMany && table.primary && table.primary.indexOf(field) !== -1) ) { continue } @@ -586,7 +594,7 @@ export class ExternalRequest { const rowId = generateIdForRow(row, table) const promise: Promise = isMany ? this.removeManyToManyRelationships(rowId, table) - : this.removeOneToManyRelationships(rowId, table, colName) + : this.removeOneToManyRelationships(rowId, table, field) if (promise) { promises.push(promise) } @@ -603,7 +611,9 @@ export class ExternalRequest { if (!isManyToOne(relationshipColumn)) { continue } - const { rows, isMany, tableId } = related[relationshipColumn.fieldName] + const { rows, isMany, tableId } = related.find( + r => r.field === relationshipColumn.fieldName + )! const table = this.getTable(tableId)! await Promise.all( rows.map(row => { From f46a6684657e043fb6bac68d83f04603e7215cd5 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 25 Oct 2024 15:55:07 +0100 Subject: [PATCH 16/38] Allow bigints to be used in view calculations, and make sure they're accurate up to 64bit signed int via tests. --- packages/backend-core/src/sql/sql.ts | 65 +++++--- .../src/api/routes/tests/viewV2.spec.ts | 144 +++++++++++++----- .../src/utilities/rowProcessor/index.ts | 27 ++-- 3 files changed, 162 insertions(+), 74 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 88953bbf99..91dceaf5a6 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -179,12 +179,6 @@ class InternalBuilder { return this.table.schema[column] } - private supportsILike(): boolean { - return !( - this.client === SqlClient.ORACLE || this.client === SqlClient.SQL_LITE - ) - } - private quoteChars(): [string, string] { const wrapped = this.knexClient.wrapIdentifier("foo", {}) return [wrapped[0], wrapped[wrapped.length - 1]] @@ -216,8 +210,30 @@ class InternalBuilder { return formatter.wrap(value, false) } - private rawQuotedValue(value: string): Knex.Raw { - return this.knex.raw(this.quotedValue(value)) + private castIntToString(identifier: string | Knex.Raw): Knex.Raw { + switch (this.client) { + case SqlClient.ORACLE: { + return this.knex.raw("to_char(??)", [identifier]) + } + case SqlClient.POSTGRES: { + return this.knex.raw("??::TEXT", [identifier]) + } + case SqlClient.MY_SQL: + case SqlClient.MARIADB: { + return this.knex.raw("CAST(?? AS CHAR)", [identifier]) + } + case SqlClient.SQL_LITE: { + // Technically sqlite can actually represent numbers larger than a 64bit + // int as a string, but it does it using scientific notation (e.g. + // "1e+20") which is not what we want. Given that the external SQL + // databases are limited to supporting only 64bit ints, we settle for + // that here. + return this.knex.raw("printf('%d', ??)", [identifier]) + } + case SqlClient.MS_SQL: { + return this.knex.raw("CONVERT(NVARCHAR, ??)", [identifier]) + } + } } // Unfortuantely we cannot rely on knex's identifier escaping because it trims @@ -1078,21 +1094,26 @@ class InternalBuilder { query = query.count(`* as ${aggregation.name}`) } } else { - const field = `${tableName}.${aggregation.field} as ${aggregation.name}` - switch (op) { - case CalculationType.SUM: - query = query.sum(field) - break - case CalculationType.AVG: - query = query.avg(field) - break - case CalculationType.MIN: - query = query.min(field) - break - case CalculationType.MAX: - query = query.max(field) - break + const fieldSchema = this.getFieldSchema(aggregation.field) + if (!fieldSchema) { + // This should not happen in practice. + throw new Error( + `field schema missing for aggregation target: ${aggregation.field}` + ) } + + let aggregate = this.knex.raw("??(??)", [ + this.knex.raw(op), + this.rawQuotedIdentifier(`${tableName}.${aggregation.field}`), + ]) + + if (fieldSchema.type === FieldType.BIGINT) { + aggregate = this.castIntToString(aggregate) + } + + query = query.select( + this.knex.raw("?? as ??", [aggregate, aggregation.name]) + ) } } return query diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 2af11b513b..44e5875b7e 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -2268,58 +2268,118 @@ describe.each([ }) }) - describe("calculation views", () => { - it("should not remove calculation columns when modifying table schema", async () => { - let table = await config.api.table.save( - saveTableRequest({ - schema: { - name: { - name: "name", - type: FieldType.STRING, + !isLucene && + describe("calculation views", () => { + it("should not remove calculation columns when modifying table schema", async () => { + let table = await config.api.table.save( + saveTableRequest({ + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + age: { + name: "age", + type: FieldType.NUMBER, + }, }, - age: { - name: "age", - type: FieldType.NUMBER, + }) + ) + + let view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + sum: { + visible: true, + calculationType: CalculationType.SUM, + field: "age", }, }, }) - ) - let view = await config.api.viewV2.create({ - tableId: table._id!, - name: generator.guid(), - type: ViewV2Type.CALCULATION, - schema: { - sum: { - visible: true, - calculationType: CalculationType.SUM, - field: "age", + table = await config.api.table.get(table._id!) + await config.api.table.save({ + ...table, + schema: { + ...table.schema, + name: { + name: "name", + type: FieldType.STRING, + constraints: { presence: true }, + }, }, - }, + }) + + view = await config.api.viewV2.get(view.id) + expect(Object.keys(view.schema!).sort()).toEqual([ + "age", + "id", + "name", + "sum", + ]) }) - table = await config.api.table.get(table._id!) - await config.api.table.save({ - ...table, - schema: { - ...table.schema, - name: { - name: "name", - type: FieldType.STRING, - constraints: { presence: true }, - }, - }, - }) + describe("bigints", () => { + let table: Table + let view: ViewV2 - view = await config.api.viewV2.get(view.id) - expect(Object.keys(view.schema!).sort()).toEqual([ - "age", - "id", - "name", - "sum", - ]) + beforeEach(async () => { + table = await config.api.table.save( + saveTableRequest({ + schema: { + bigint: { + name: "bigint", + type: FieldType.BIGINT, + }, + }, + }) + ) + + view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + sum: { + visible: true, + calculationType: CalculationType.SUM, + field: "bigint", + }, + }, + }) + }) + + it("should not lose precision handling ints larger than JSs int53", async () => { + // The sum of the following 3 numbers cannot be represented by + // JavaScripts default int53 datatype for numbers, so this is a test + // that makes sure we aren't losing precision between the DB and the + // user. + await config.api.row.bulkImport(table._id!, { + rows: [ + { bigint: "1000000000000000000" }, + { bigint: "123" }, + { bigint: "321" }, + ], + }) + + const { rows } = await config.api.row.search(view.id) + expect(rows).toHaveLength(1) + expect(rows[0].sum).toEqual("1000000000000000444") + }) + + it("should be able to handle up to 2**63 - 1 bigints", async () => { + await config.api.row.bulkImport(table._id!, { + rows: [{ bigint: "9223372036854775806" }, { bigint: "1" }], + }) + + const { rows } = await config.api.row.search(view.id) + expect(rows).toHaveLength(1) + expect(rows[0].sum).toEqual("9223372036854775807") + }) + }) }) - }) }) describe("row operations", () => { diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index b9f4d0db8b..0ffa7600aa 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -440,19 +440,26 @@ export async function coreOutputProcessing( } if (sdk.views.isView(source)) { - const calculationFields = Object.keys( - helpers.views.calculationFields(source) - ) - - // We ensure all calculation fields are returned as numbers. During the + // We ensure calculation fields are returned as numbers. During the // testing of this feature it was discovered that the COUNT operation // returns a string for MySQL, MariaDB, and Postgres. But given that all - // calculation fields should be numbers, we blanket make sure of that - // here. - for (const key of calculationFields) { + // calculation fields (except ones operating on BIGINTs) should be + // numbers, we blanket make sure of that here. + for (const [name, field] of Object.entries( + helpers.views.calculationFields(source) + )) { + if ("field" in field) { + const targetSchema = table.schema[field.field] + // We don't convert BIGINT fields to floats because we could lose + // precision. + if (targetSchema.type === FieldType.BIGINT) { + continue + } + } + for (const row of rows) { - if (typeof row[key] === "string") { - row[key] = parseFloat(row[key]) + if (typeof row[name] === "string") { + row[name] = parseFloat(row[name]) } } } From 00eedbf72685d6a8a24f3b9a946f054d9a0de8e3 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 25 Oct 2024 16:57:50 +0200 Subject: [PATCH 17/38] Fix deletions --- .../api/controllers/row/ExternalRequest.ts | 27 ++++---- .../server/src/api/routes/tests/row.spec.ts | 66 +++++++++++++++++++ 2 files changed, 82 insertions(+), 11 deletions(-) diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index 527bc4b3ef..1fb8ce0423 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -11,6 +11,7 @@ import { IncludeRelationship, InternalSearchFilterOperator, isManyToOne, + isOneToMany, OneToManyRelationshipFieldMetadata, Operation, PaginationJson, @@ -50,6 +51,7 @@ import sdk from "../../../sdk" import env from "../../../environment" import { makeExternalQuery } from "../../../integrations/base/query" import { dataFilters, helpers } from "@budibase/shared-core" +import { isRelationshipColumn } from "../../../db/utils" export interface ManyRelationship { tableId?: string @@ -606,25 +608,28 @@ export class ExternalRequest { async removeRelationshipsToRow(table: Table, rowId: string) { const row = await this.getRow(table, rowId) const related = await this.lookupRelations(table._id!, row) - for (let column of Object.values(table.schema)) { - const relationshipColumn = column as RelationshipFieldMetadata - if (!isManyToOne(relationshipColumn)) { + for (const column of Object.values(table.schema)) { + if (!isRelationshipColumn(column) || isOneToMany(column)) { continue } - const { rows, isMany, tableId } = related.find( - r => r.field === relationshipColumn.fieldName - )! + + const relatedByTable = isManyToMany(column) + ? related.find( + r => r.tableId === column.through && r.field === column.fieldName + ) + : related.find(r => r.field === column.fieldName) + if (!relatedByTable) { + continue + } + + const { rows, isMany, tableId } = relatedByTable const table = this.getTable(tableId)! await Promise.all( rows.map(row => { const rowId = generateIdForRow(row, table) return isMany ? this.removeManyToManyRelationships(rowId, table) - : this.removeOneToManyRelationships( - rowId, - table, - relationshipColumn.fieldName - ) + : this.removeOneToManyRelationships(rowId, table, column.fieldName) }) ) } diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 32788d2783..19d6595619 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -1542,6 +1542,72 @@ describe.each([ ) expect(res.length).toEqual(2) }) + + describe("relations to same table", () => { + let relatedRows: Row[] + + beforeAll(async () => { + const relatedTable = await config.api.table.save( + defaultTable({ + schema: { + name: { name: "name", type: FieldType.STRING }, + }, + }) + ) + const relatedTableId = relatedTable._id! + table = await config.api.table.save( + defaultTable({ + schema: { + name: { name: "name", type: FieldType.STRING }, + related1: { + type: FieldType.LINK, + name: "related1", + fieldName: "main1", + tableId: relatedTableId, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + related2: { + type: FieldType.LINK, + name: "related2", + fieldName: "main2", + tableId: relatedTableId, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + }, + }) + ) + relatedRows = await Promise.all([ + config.api.row.save(relatedTableId, { name: "foo" }), + config.api.row.save(relatedTableId, { name: "bar" }), + config.api.row.save(relatedTableId, { name: "baz" }), + config.api.row.save(relatedTableId, { name: "boo" }), + ]) + }) + + it("can delete rows with both relationships", async () => { + const row = await config.api.row.save(table._id!, { + name: "test", + related1: [relatedRows[0]._id!], + related2: [relatedRows[1]._id!], + }) + + await config.api.row.delete(table._id!, { _id: row._id! }) + + await config.api.row.get(table._id!, row._id!, { status: 404 }) + }) + + it("can delete rows with empty relationships", async () => { + const row = await config.api.row.save(table._id!, { + name: "test", + related1: [], + related2: [], + }) + + await config.api.row.delete(table._id!, { _id: row._id! }) + + await config.api.row.get(table._id!, row._id!, { status: 404 }) + }) + }) }) describe("validate", () => { From a01c5ed654dbc3100522bc5267269b406a317e97 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 25 Oct 2024 17:01:25 +0200 Subject: [PATCH 18/38] Don't run tests for lucene --- .../server/src/api/routes/tests/row.spec.ts | 495 +++++++++--------- 1 file changed, 249 insertions(+), 246 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 19d6595619..19f11a044a 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -940,103 +940,104 @@ describe.each([ }) }) - describe("relations to same table", () => { - let relatedRows: Row[] + !isLucene && + describe("relations to same table", () => { + let relatedRows: Row[] - beforeAll(async () => { - const relatedTable = await config.api.table.save( - defaultTable({ - schema: { - name: { name: "name", type: FieldType.STRING }, - }, - }) - ) - const relatedTableId = relatedTable._id! - table = await config.api.table.save( - defaultTable({ - schema: { - name: { name: "name", type: FieldType.STRING }, - related1: { - type: FieldType.LINK, - name: "related1", - fieldName: "main1", - tableId: relatedTableId, - relationshipType: RelationshipType.MANY_TO_MANY, + beforeAll(async () => { + const relatedTable = await config.api.table.save( + defaultTable({ + schema: { + name: { name: "name", type: FieldType.STRING }, }, - related2: { - type: FieldType.LINK, - name: "related2", - fieldName: "main2", - tableId: relatedTableId, - relationshipType: RelationshipType.MANY_TO_MANY, + }) + ) + const relatedTableId = relatedTable._id! + table = await config.api.table.save( + defaultTable({ + schema: { + name: { name: "name", type: FieldType.STRING }, + related1: { + type: FieldType.LINK, + name: "related1", + fieldName: "main1", + tableId: relatedTableId, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + related2: { + type: FieldType.LINK, + name: "related2", + fieldName: "main2", + tableId: relatedTableId, + relationshipType: RelationshipType.MANY_TO_MANY, + }, }, - }, - }) - ) - relatedRows = await Promise.all([ - config.api.row.save(relatedTableId, { name: "foo" }), - config.api.row.save(relatedTableId, { name: "bar" }), - config.api.row.save(relatedTableId, { name: "baz" }), - config.api.row.save(relatedTableId, { name: "boo" }), - ]) - }) - - it("can create rows with both relationships", async () => { - const row = await config.api.row.save(table._id!, { - name: "test", - related1: [relatedRows[0]._id!], - related2: [relatedRows[1]._id!], + }) + ) + relatedRows = await Promise.all([ + config.api.row.save(relatedTableId, { name: "foo" }), + config.api.row.save(relatedTableId, { name: "bar" }), + config.api.row.save(relatedTableId, { name: "baz" }), + config.api.row.save(relatedTableId, { name: "boo" }), + ]) }) - expect(row).toEqual( - expect.objectContaining({ + it("can create rows with both relationships", async () => { + const row = await config.api.row.save(table._id!, { name: "test", - related1: [ - { - _id: relatedRows[0]._id, - primaryDisplay: relatedRows[0].name, - }, - ], - related2: [ - { - _id: relatedRows[1]._id, - primaryDisplay: relatedRows[1].name, - }, - ], + related1: [relatedRows[0]._id!], + related2: [relatedRows[1]._id!], }) - ) - }) - it("can create rows with no relationships", async () => { - const row = await config.api.row.save(table._id!, { - name: "test", + expect(row).toEqual( + expect.objectContaining({ + name: "test", + related1: [ + { + _id: relatedRows[0]._id, + primaryDisplay: relatedRows[0].name, + }, + ], + related2: [ + { + _id: relatedRows[1]._id, + primaryDisplay: relatedRows[1].name, + }, + ], + }) + ) }) - expect(row.related1).toBeUndefined() - expect(row.related2).toBeUndefined() - }) - - it("can create rows with only one relationships field", async () => { - const row = await config.api.row.save(table._id!, { - name: "test", - related1: [], - related2: [relatedRows[1]._id!], - }) - - expect(row).toEqual( - expect.objectContaining({ + it("can create rows with no relationships", async () => { + const row = await config.api.row.save(table._id!, { name: "test", - related2: [ - { - _id: relatedRows[1]._id, - primaryDisplay: relatedRows[1].name, - }, - ], }) - ) - expect(row.related1).toBeUndefined() + + expect(row.related1).toBeUndefined() + expect(row.related2).toBeUndefined() + }) + + it("can create rows with only one relationships field", async () => { + const row = await config.api.row.save(table._id!, { + name: "test", + related1: [], + related2: [relatedRows[1]._id!], + }) + + expect(row).toEqual( + expect.objectContaining({ + name: "test", + related2: [ + { + _id: relatedRows[1]._id, + primaryDisplay: relatedRows[1].name, + }, + ], + }) + ) + expect(row.related1).toBeUndefined() + }) }) - }) }) describe("get", () => { @@ -1140,132 +1141,133 @@ describe.each([ expect(rows).toHaveLength(1) }) - describe("relations to same table", () => { - let relatedRows: Row[] + !isLucene && + describe("relations to same table", () => { + let relatedRows: Row[] - beforeAll(async () => { - const relatedTable = await config.api.table.save( - defaultTable({ - schema: { - name: { name: "name", type: FieldType.STRING }, - }, - }) - ) - const relatedTableId = relatedTable._id! - table = await config.api.table.save( - defaultTable({ - schema: { - name: { name: "name", type: FieldType.STRING }, - related1: { - type: FieldType.LINK, - name: "related1", - fieldName: "main1", - tableId: relatedTableId, - relationshipType: RelationshipType.MANY_TO_MANY, + beforeAll(async () => { + const relatedTable = await config.api.table.save( + defaultTable({ + schema: { + name: { name: "name", type: FieldType.STRING }, }, - related2: { - type: FieldType.LINK, - name: "related2", - fieldName: "main2", - tableId: relatedTableId, - relationshipType: RelationshipType.MANY_TO_MANY, + }) + ) + const relatedTableId = relatedTable._id! + table = await config.api.table.save( + defaultTable({ + schema: { + name: { name: "name", type: FieldType.STRING }, + related1: { + type: FieldType.LINK, + name: "related1", + fieldName: "main1", + tableId: relatedTableId, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + related2: { + type: FieldType.LINK, + name: "related2", + fieldName: "main2", + tableId: relatedTableId, + relationshipType: RelationshipType.MANY_TO_MANY, + }, }, - }, - }) - ) - relatedRows = await Promise.all([ - config.api.row.save(relatedTableId, { name: "foo" }), - config.api.row.save(relatedTableId, { name: "bar" }), - config.api.row.save(relatedTableId, { name: "baz" }), - config.api.row.save(relatedTableId, { name: "boo" }), - ]) - }) - - it("can edit rows with both relationships", async () => { - let row = await config.api.row.save(table._id!, { - name: "test", - related1: [relatedRows[0]._id!], - related2: [relatedRows[1]._id!], + }) + ) + relatedRows = await Promise.all([ + config.api.row.save(relatedTableId, { name: "foo" }), + config.api.row.save(relatedTableId, { name: "bar" }), + config.api.row.save(relatedTableId, { name: "baz" }), + config.api.row.save(relatedTableId, { name: "boo" }), + ]) }) - row = await config.api.row.save(table._id!, { - ...row, - related1: [relatedRows[0]._id!, relatedRows[1]._id!], - related2: [relatedRows[2]._id!], - }) - - expect(row).toEqual( - expect.objectContaining({ + it("can edit rows with both relationships", async () => { + let row = await config.api.row.save(table._id!, { name: "test", - related1: [ - { - _id: relatedRows[1]._id, - primaryDisplay: relatedRows[1].name, - }, - { - _id: relatedRows[0]._id, - primaryDisplay: relatedRows[0].name, - }, - ], - related2: [ - { - _id: relatedRows[2]._id, - primaryDisplay: relatedRows[2].name, - }, - ], + related1: [relatedRows[0]._id!], + related2: [relatedRows[1]._id!], }) - ) - }) - it("can drop existing relationship", async () => { - let row = await config.api.row.save(table._id!, { - name: "test", - related1: [relatedRows[0]._id!], - related2: [relatedRows[1]._id!], + row = await config.api.row.save(table._id!, { + ...row, + related1: [relatedRows[0]._id!, relatedRows[1]._id!], + related2: [relatedRows[2]._id!], + }) + + expect(row).toEqual( + expect.objectContaining({ + name: "test", + related1: [ + { + _id: relatedRows[1]._id, + primaryDisplay: relatedRows[1].name, + }, + { + _id: relatedRows[0]._id, + primaryDisplay: relatedRows[0].name, + }, + ], + related2: [ + { + _id: relatedRows[2]._id, + primaryDisplay: relatedRows[2].name, + }, + ], + }) + ) }) - row = await config.api.row.save(table._id!, { - ...row, - related1: [], - related2: [relatedRows[2]._id!], - }) - - expect(row).toEqual( - expect.objectContaining({ + it("can drop existing relationship", async () => { + let row = await config.api.row.save(table._id!, { name: "test", - related2: [ - { - _id: relatedRows[2]._id, - primaryDisplay: relatedRows[2].name, - }, - ], + related1: [relatedRows[0]._id!], + related2: [relatedRows[1]._id!], }) - ) - expect(row.related1).toBeUndefined() - }) - it("can drop both relationships", async () => { - let row = await config.api.row.save(table._id!, { - name: "test", - related1: [relatedRows[0]._id!], - related2: [relatedRows[1]._id!], + row = await config.api.row.save(table._id!, { + ...row, + related1: [], + related2: [relatedRows[2]._id!], + }) + + expect(row).toEqual( + expect.objectContaining({ + name: "test", + related2: [ + { + _id: relatedRows[2]._id, + primaryDisplay: relatedRows[2].name, + }, + ], + }) + ) + expect(row.related1).toBeUndefined() }) - row = await config.api.row.save(table._id!, { - ...row, - related1: [], - related2: [], - }) - - expect(row).toEqual( - expect.objectContaining({ + it("can drop both relationships", async () => { + let row = await config.api.row.save(table._id!, { name: "test", + related1: [relatedRows[0]._id!], + related2: [relatedRows[1]._id!], }) - ) - expect(row.related1).toBeUndefined() - expect(row.related2).toBeUndefined() + + row = await config.api.row.save(table._id!, { + ...row, + related1: [], + related2: [], + }) + + expect(row).toEqual( + expect.objectContaining({ + name: "test", + }) + ) + expect(row.related1).toBeUndefined() + expect(row.related2).toBeUndefined() + }) }) - }) }) describe("patch", () => { @@ -1543,71 +1545,72 @@ describe.each([ expect(res.length).toEqual(2) }) - describe("relations to same table", () => { - let relatedRows: Row[] + !isLucene && + describe("relations to same table", () => { + let relatedRows: Row[] - beforeAll(async () => { - const relatedTable = await config.api.table.save( - defaultTable({ - schema: { - name: { name: "name", type: FieldType.STRING }, - }, - }) - ) - const relatedTableId = relatedTable._id! - table = await config.api.table.save( - defaultTable({ - schema: { - name: { name: "name", type: FieldType.STRING }, - related1: { - type: FieldType.LINK, - name: "related1", - fieldName: "main1", - tableId: relatedTableId, - relationshipType: RelationshipType.MANY_TO_MANY, + beforeAll(async () => { + const relatedTable = await config.api.table.save( + defaultTable({ + schema: { + name: { name: "name", type: FieldType.STRING }, }, - related2: { - type: FieldType.LINK, - name: "related2", - fieldName: "main2", - tableId: relatedTableId, - relationshipType: RelationshipType.MANY_TO_MANY, + }) + ) + const relatedTableId = relatedTable._id! + table = await config.api.table.save( + defaultTable({ + schema: { + name: { name: "name", type: FieldType.STRING }, + related1: { + type: FieldType.LINK, + name: "related1", + fieldName: "main1", + tableId: relatedTableId, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + related2: { + type: FieldType.LINK, + name: "related2", + fieldName: "main2", + tableId: relatedTableId, + relationshipType: RelationshipType.MANY_TO_MANY, + }, }, - }, - }) - ) - relatedRows = await Promise.all([ - config.api.row.save(relatedTableId, { name: "foo" }), - config.api.row.save(relatedTableId, { name: "bar" }), - config.api.row.save(relatedTableId, { name: "baz" }), - config.api.row.save(relatedTableId, { name: "boo" }), - ]) - }) - - it("can delete rows with both relationships", async () => { - const row = await config.api.row.save(table._id!, { - name: "test", - related1: [relatedRows[0]._id!], - related2: [relatedRows[1]._id!], + }) + ) + relatedRows = await Promise.all([ + config.api.row.save(relatedTableId, { name: "foo" }), + config.api.row.save(relatedTableId, { name: "bar" }), + config.api.row.save(relatedTableId, { name: "baz" }), + config.api.row.save(relatedTableId, { name: "boo" }), + ]) }) - await config.api.row.delete(table._id!, { _id: row._id! }) + it("can delete rows with both relationships", async () => { + const row = await config.api.row.save(table._id!, { + name: "test", + related1: [relatedRows[0]._id!], + related2: [relatedRows[1]._id!], + }) - await config.api.row.get(table._id!, row._id!, { status: 404 }) - }) + await config.api.row.delete(table._id!, { _id: row._id! }) - it("can delete rows with empty relationships", async () => { - const row = await config.api.row.save(table._id!, { - name: "test", - related1: [], - related2: [], + await config.api.row.get(table._id!, row._id!, { status: 404 }) }) - await config.api.row.delete(table._id!, { _id: row._id! }) + it("can delete rows with empty relationships", async () => { + const row = await config.api.row.save(table._id!, { + name: "test", + related1: [], + related2: [], + }) - await config.api.row.get(table._id!, row._id!, { status: 404 }) + await config.api.row.delete(table._id!, { _id: row._id! }) + + await config.api.row.get(table._id!, row._id!, { status: 404 }) + }) }) - }) }) describe("validate", () => { From 407484bbe53e7ed05e3a2381b1fdd98547a21def Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 25 Oct 2024 16:21:10 +0100 Subject: [PATCH 19/38] Respond to PR comments. --- packages/server/src/threads/automation.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/server/src/threads/automation.ts b/packages/server/src/threads/automation.ts index 7083562006..1af8646cf4 100644 --- a/packages/server/src/threads/automation.ts +++ b/packages/server/src/threads/automation.ts @@ -263,11 +263,16 @@ class Orchestrator { this.context.env = await sdkUtils.getEnvironmentVariables() this.context.user = this.currentUser - const { config } = await configs.getSettingsConfigDoc() - this.context.settings = { - url: config.platformUrl, - logo: config.logoUrl, - company: config.company, + try { + const { config } = await configs.getSettingsConfigDoc() + this.context.settings = { + url: config.platformUrl, + logo: config.logoUrl, + company: config.company, + } + } catch (e) { + // if settings doc doesn't exist, make the settings blank + this.context.settings = {} } let metadata From 9ab266a96368fbd68666695931ad20517d780263 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 25 Oct 2024 16:38:24 +0100 Subject: [PATCH 20/38] Fix lint. --- .../buttons/grid/GridViewCalculationButton.svelte | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte b/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte index db58ba4a8f..87c77a1457 100644 --- a/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte +++ b/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte @@ -7,12 +7,7 @@ Icon, Multiselect, } from "@budibase/bbui" - import { - CalculationType, - canGroupBy, - FieldType, - isNumeric, - } from "@budibase/types" + import { CalculationType, canGroupBy, isNumeric } from "@budibase/types" import InfoDisplay from "pages/builder/app/[application]/design/[screenId]/[componentId]/_components/Component/InfoDisplay.svelte" import { getContext } from "svelte" From 708b1d652739691466be8c311ba18b3e19f9b13d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 28 Oct 2024 10:37:25 +0100 Subject: [PATCH 21/38] Update yarn.lock --- yarn.lock | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/yarn.lock b/yarn.lock index 1198e98ad6..9810b76ef3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2051,7 +2051,7 @@ resolved "https://registry.yarnpkg.com/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39" integrity sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw== -"@budibase/backend-core@2.32.11": +"@budibase/backend-core@2.33.2": version "0.0.0" dependencies: "@budibase/nano" "10.1.5" @@ -2132,15 +2132,15 @@ through2 "^2.0.0" "@budibase/pro@npm:@budibase/pro@latest": - version "2.32.11" - resolved "https://registry.yarnpkg.com/@budibase/pro/-/pro-2.32.11.tgz#c94d534f829ca0ef252677757e157a7e58b87b4d" - integrity sha512-mOkqJpqHKWsfTWZwWcvBCYFUIluSUHltQNinc1ZRsg9rC3OKoHSDop6gzm744++H/GzGRN8V86kLhCgtNIlkpA== + version "2.33.2" + resolved "https://registry.yarnpkg.com/@budibase/pro/-/pro-2.33.2.tgz#5c2012f7b2bf0fd871cda1ad37ad7a0442c84658" + integrity sha512-lBB6Wfp6OIOHRlGq82WS9KxvEXRs/P2QlwJT0Aj9PhmkQFsnXm2r8d18f0xTGvcflD+iR7XGP/k56JlCanmhQg== dependencies: "@anthropic-ai/sdk" "^0.27.3" - "@budibase/backend-core" "2.32.11" - "@budibase/shared-core" "2.32.11" - "@budibase/string-templates" "2.32.11" - "@budibase/types" "2.32.11" + "@budibase/backend-core" "2.33.2" + "@budibase/shared-core" "2.33.2" + "@budibase/string-templates" "2.33.2" + "@budibase/types" "2.33.2" "@koa/router" "8.0.8" bull "4.10.1" dd-trace "5.2.0" @@ -2153,13 +2153,13 @@ scim-patch "^0.8.1" scim2-parse-filter "^0.2.8" -"@budibase/shared-core@2.32.11": +"@budibase/shared-core@2.33.2": version "0.0.0" dependencies: "@budibase/types" "0.0.0" cron-validate "1.4.5" -"@budibase/string-templates@2.32.11": +"@budibase/string-templates@2.33.2": version "0.0.0" dependencies: "@budibase/handlebars-helpers" "^0.13.2" @@ -2167,7 +2167,7 @@ handlebars "^4.7.8" lodash.clonedeep "^4.5.0" -"@budibase/types@2.32.11": +"@budibase/types@2.33.2": version "0.0.0" dependencies: scim-patch "^0.8.1" From 8aeb19aabe0058e7ab895528198c582ae4f101d5 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 28 Oct 2024 10:54:00 +0100 Subject: [PATCH 22/38] Fix flakiness --- packages/server/src/api/routes/tests/row.spec.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 19f11a044a..72ade0a69a 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -1199,16 +1199,16 @@ describe.each([ expect(row).toEqual( expect.objectContaining({ name: "test", - related1: [ - { - _id: relatedRows[1]._id, - primaryDisplay: relatedRows[1].name, - }, + related1: expect.arrayContaining([ { _id: relatedRows[0]._id, primaryDisplay: relatedRows[0].name, }, - ], + { + _id: relatedRows[1]._id, + primaryDisplay: relatedRows[1].name, + }, + ]), related2: [ { _id: relatedRows[2]._id, From 95de01c86c8971a745eaf0f2107ed17e5213da30 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 28 Oct 2024 13:17:07 +0100 Subject: [PATCH 23/38] Simplifying lookups --- .../api/controllers/row/ExternalRequest.ts | 49 ++++++++++++------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index 1fb8ce0423..c5ebe69aea 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -17,6 +17,7 @@ import { PaginationJson, QueryJson, RelationshipFieldMetadata, + RelationshipType, Row, SearchFilters, SortJson, @@ -421,17 +422,30 @@ export class ExternalRequest { return { row: newRow as T, manyRelationships } } + private getLookupRelationsKey(relationship: { + relationshipType: RelationshipType + fieldName: string + through?: string + }) { + if (relationship.relationshipType === RelationshipType.MANY_TO_MANY) { + return `${relationship.through}_${relationship.fieldName}` + } + return relationship.fieldName + } /** * This is a cached lookup, of relationship records, this is mainly for creating/deleting junction * information. */ - async lookupRelations(tableId: string, row: Row) { - const related: { - rows: Row[] - isMany: boolean - tableId: string - field: string - }[] = [] + private async lookupRelations(tableId: string, row: Row) { + const related: Record< + string, + { + rows: Row[] + isMany: boolean + tableId: string + field: string + } + > = {} const { tableName } = breakExternalTableId(tableId) const table = this.tables[tableName] @@ -496,12 +510,12 @@ export class ExternalRequest { ? field.throughFrom || linkPrimaryKey : fieldName - related.push({ + related[this.getLookupRelationsKey(field)] = { rows, isMany: isManyToMany(field), tableId: relatedTableId, field: storeTo, - }) + } } return related } @@ -537,8 +551,13 @@ export class ExternalRequest { const linkSecondary = relationshipPrimary[1] const rows = - related.find(r => r.tableId === relationship.tableId && r.field === key) - ?.rows || [] + related[ + this.getLookupRelationsKey({ + relationshipType: RelationshipType.MANY_TO_MANY, + fieldName: key, + through: relationship.tableId, + }) + ]?.rows || [] const relationshipMatchPredicate = ({ row, @@ -583,7 +602,7 @@ export class ExternalRequest { } } // finally cleanup anything that needs to be removed - for (let { isMany, rows, tableId, field } of related) { + for (let { isMany, rows, tableId, field } of Object.values(related)) { const table: Table | undefined = this.getTable(tableId) // if it's not the foreign key skip it, nothing to do if ( @@ -613,11 +632,7 @@ export class ExternalRequest { continue } - const relatedByTable = isManyToMany(column) - ? related.find( - r => r.tableId === column.through && r.field === column.fieldName - ) - : related.find(r => r.field === column.fieldName) + const relatedByTable = related[this.getLookupRelationsKey(column)] if (!relatedByTable) { continue } From d944e3a8f9c3a73c2b23c75b7a69b191ab955cb2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 28 Oct 2024 13:53:44 +0100 Subject: [PATCH 24/38] Cleanup --- .../src/api/controllers/row/ExternalRequest.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index c5ebe69aea..49f2f02ebe 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -443,7 +443,6 @@ export class ExternalRequest { rows: Row[] isMany: boolean tableId: string - field: string } > = {} @@ -478,10 +477,7 @@ export class ExternalRequest { "Unable to lookup relationships - undefined column properties." ) } - const { tableName: relatedTableName } = - breakExternalTableId(relatedTableId) - // @ts-ignore - const linkPrimaryKey = this.tables[relatedTableName].primary[0] + if (!lookupField || !row?.[lookupField] == null) { continue } @@ -506,15 +502,11 @@ export class ExternalRequest { !Array.isArray(response) || isKnexEmptyReadResponse(response) ? [] : response - const storeTo = isManyToMany(field) - ? field.throughFrom || linkPrimaryKey - : fieldName related[this.getLookupRelationsKey(field)] = { rows, isMany: isManyToMany(field), tableId: relatedTableId, - field: storeTo, } } return related @@ -602,7 +594,7 @@ export class ExternalRequest { } } // finally cleanup anything that needs to be removed - for (let { isMany, rows, tableId, field } of Object.values(related)) { + for (const [field, { isMany, rows, tableId }] of Object.entries(related)) { const table: Table | undefined = this.getTable(tableId) // if it's not the foreign key skip it, nothing to do if ( From b92427015b1ae0634e56763605a18d286545bc10 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 28 Oct 2024 13:55:08 +0100 Subject: [PATCH 25/38] Cleanup test --- packages/server/src/api/routes/tests/row.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 72ade0a69a..a11f1d758a 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -3076,7 +3076,7 @@ describe.each([ }, ], ["from original saved row", (row: Row) => row], - ["from updated row", (row: Row) => config.api.row.save(viewId, row)], + ["from updated row", (row: Row) => config.api.row.save(viewId, row)], ] it.each(testScenarios)( From 641ceea92e1baf5f1279e5f67b2273cd58849da4 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 28 Oct 2024 13:57:53 +0100 Subject: [PATCH 26/38] Fix checks --- packages/server/src/api/controllers/row/ExternalRequest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index 49f2f02ebe..07331bd701 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -478,7 +478,7 @@ export class ExternalRequest { ) } - if (!lookupField || !row?.[lookupField] == null) { + if (!lookupField || !row?.[lookupField]) { continue } const endpoint = getEndpoint(relatedTableId, Operation.READ) From d36b810efcbbd6fe1c1df253d6859f8c1ee26fb3 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 28 Oct 2024 14:28:53 +0100 Subject: [PATCH 27/38] Fix flakiness --- .../server/src/api/controllers/row/ExternalRequest.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index 07331bd701..b884683197 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -54,12 +54,13 @@ import { makeExternalQuery } from "../../../integrations/base/query" import { dataFilters, helpers } from "@budibase/shared-core" import { isRelationshipColumn } from "../../../db/utils" -export interface ManyRelationship { +interface ManyRelationship { tableId?: string id?: string isUpdate?: boolean key: string [key: string]: any + relationshipType: RelationshipType } export interface RunConfig { @@ -386,6 +387,7 @@ export class ExternalRequest { [otherKey]: breakRowIdField(relationship)[0], // leave the ID for enrichment later [thisKey]: `{{ literal ${tablePrimary} }}`, + relationshipType: RelationshipType.MANY_TO_MANY, }) } } @@ -402,6 +404,7 @@ export class ExternalRequest { [thisKey]: breakRowIdField(relationship)[0], // leave the ID for enrichment later [otherKey]: `{{ literal ${tablePrimary} }}`, + relationshipType: RelationshipType.MANY_TO_ONE, }) } } @@ -531,7 +534,8 @@ export class ExternalRequest { const promises = [] const related = await this.lookupRelations(mainTableId, row) for (let relationship of relationships) { - const { key, tableId, isUpdate, id, ...rest } = relationship + const { key, tableId, isUpdate, id, relationshipType, ...rest } = + relationship const body: { [key: string]: any } = processObjectSync(rest, row, {}) const linkTable = this.getTable(tableId) const relationshipPrimary = linkTable?.primary || [] @@ -545,7 +549,7 @@ export class ExternalRequest { const rows = related[ this.getLookupRelationsKey({ - relationshipType: RelationshipType.MANY_TO_MANY, + relationshipType, fieldName: key, through: relationship.tableId, }) From 184df942791d9aa0dac781353f97788a1945793e Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Mon, 28 Oct 2024 13:35:25 +0000 Subject: [PATCH 28/38] Bump version to 2.33.3 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index df50828b30..8235ee9b60 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.33.2", + "version": "2.33.3", "npmClient": "yarn", "packages": [ "packages/*", From 7dcdce24807c439102c1c6f270f2790ff2990a48 Mon Sep 17 00:00:00 2001 From: melohagan <101575380+melohagan@users.noreply.github.com> Date: Mon, 28 Oct 2024 14:46:42 +0000 Subject: [PATCH 29/38] Gro 738 refactor code that returns the account holder from user (#14872) * WIP * WIP * Remove tests * Remove TenantInfo * Remove unused export * Remove TenantInfo type * Remove unused export * Remove unused routes * Add getAccountHolder front-end endpoint * Add endpoint to no tenancy list * Get account holder via appId * lint * Update pro ref * Use account email instead of budibaseUserId (#14876) * Update account-portal ref * Update account portal ref * Correct import order * Simplify boolean * type fix * Rename endpoint to accountholder * Update account-portal ref * Refactor * Refactor to not use appId * Update type * appId not needed * Unused import --- packages/account-portal | 2 +- .../backend-core/src/events/identification.ts | 13 ++-- packages/backend-core/src/tenancy/db.ts | 23 ------ packages/backend-core/src/users/db.ts | 21 ++--- packages/backend-core/src/users/users.ts | 11 +++ packages/backend-core/src/users/utils.ts | 30 +++----- .../portal/users/users/[userId].svelte | 2 +- .../builder/portal/users/users/index.svelte | 9 ++- packages/builder/src/stores/portal/users.js | 11 ++- packages/frontend-core/src/api/user.js | 10 +-- packages/types/src/api/web/user.ts | 5 +- packages/types/src/documents/global/index.ts | 1 - .../types/src/documents/global/tenantInfo.ts | 15 ---- packages/types/src/documents/global/user.ts | 5 ++ .../src/api/controllers/global/tenant.ts | 14 ---- .../src/api/controllers/global/users.ts | 34 +++++++-- packages/worker/src/api/index.ts | 10 +-- .../worker/src/api/routes/global/tenant.ts | 11 --- .../api/routes/global/tests/tenant.spec.ts | 48 ------------ .../src/api/routes/global/tests/users.spec.ts | 76 +++---------------- .../worker/src/api/routes/global/users.ts | 1 + packages/worker/src/api/routes/index.ts | 2 - .../worker/src/api/routes/validation/users.ts | 9 ++- packages/worker/src/tests/api/tenants.ts | 9 --- packages/worker/src/tests/api/users.ts | 10 ++- 25 files changed, 126 insertions(+), 256 deletions(-) delete mode 100644 packages/types/src/documents/global/tenantInfo.ts delete mode 100644 packages/worker/src/api/controllers/global/tenant.ts delete mode 100644 packages/worker/src/api/routes/global/tenant.ts delete mode 100644 packages/worker/src/api/routes/global/tests/tenant.spec.ts diff --git a/packages/account-portal b/packages/account-portal index 8cd052ce82..607e3db866 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit 8cd052ce8288f343812a514d06c5a9459b3ba1a8 +Subproject commit 607e3db866c366cb609b0015a1293edbeb703237 diff --git a/packages/backend-core/src/events/identification.ts b/packages/backend-core/src/events/identification.ts index c7bc1c817b..69bf7009b2 100644 --- a/packages/backend-core/src/events/identification.ts +++ b/packages/backend-core/src/events/identification.ts @@ -171,9 +171,9 @@ const identifyUser = async ( if (isSSOUser(user)) { providerType = user.providerType } - const accountHolder = account?.budibaseUserId === user._id || false - const verified = - account && account?.budibaseUserId === user._id ? account.verified : false + const accountHolder = await users.getExistingAccounts([user.email]) + const isAccountHolder = accountHolder.length > 0 + const verified = !!account && isAccountHolder && account.verified const installationId = await getInstallationId() const hosting = account ? account.hosting : getHostingFromEnv() const environment = getDeploymentEnvironment() @@ -185,7 +185,7 @@ const identifyUser = async ( installationId, tenantId, verified, - accountHolder, + accountHolder: isAccountHolder, providerType, builder, admin, @@ -207,9 +207,10 @@ const identifyAccount = async (account: Account) => { const environment = getDeploymentEnvironment() if (isCloudAccount(account)) { - if (account.budibaseUserId) { + const user = await users.getGlobalUserByEmail(account.email) + if (user?._id) { // use the budibase user as the id if set - id = account.budibaseUserId + id = user._id } } diff --git a/packages/backend-core/src/tenancy/db.ts b/packages/backend-core/src/tenancy/db.ts index 332ecbca48..10477a8579 100644 --- a/packages/backend-core/src/tenancy/db.ts +++ b/packages/backend-core/src/tenancy/db.ts @@ -1,29 +1,6 @@ import { getDB } from "../db/db" import { getGlobalDBName } from "../context" -import { TenantInfo } from "@budibase/types" export function getTenantDB(tenantId: string) { return getDB(getGlobalDBName(tenantId)) } - -export async function saveTenantInfo(tenantInfo: TenantInfo) { - const db = getTenantDB(tenantInfo.tenantId) - // save the tenant info to db - return db.put({ - _id: "tenant_info", - ...tenantInfo, - }) -} - -export async function getTenantInfo( - tenantId: string -): Promise { - try { - const db = getTenantDB(tenantId) - const tenantInfo = (await db.get("tenant_info")) as TenantInfo - delete tenantInfo.owner.password - return tenantInfo - } catch { - return undefined - } -} diff --git a/packages/backend-core/src/users/db.ts b/packages/backend-core/src/users/db.ts index c96c615f4b..cbc0019303 100644 --- a/packages/backend-core/src/users/db.ts +++ b/packages/backend-core/src/users/db.ts @@ -16,14 +16,15 @@ import { isSSOUser, SaveUserOpts, User, - UserStatus, UserGroup, + UserIdentifier, + UserStatus, PlatformUserBySsoId, PlatformUserById, AnyDocument, } from "@budibase/types" import { - getAccountHolderFromUserIds, + getAccountHolderFromUsers, isAdmin, isCreator, validateUniqueUser, @@ -412,7 +413,9 @@ export class UserDB { ) } - static async bulkDelete(userIds: string[]): Promise { + static async bulkDelete( + users: Array + ): Promise { const db = getGlobalDB() const response: BulkUserDeleted = { @@ -421,13 +424,13 @@ export class UserDB { } // remove the account holder from the delete request if present - const account = await getAccountHolderFromUserIds(userIds) - if (account) { - userIds = userIds.filter(u => u !== account.budibaseUserId) + const accountHolder = await getAccountHolderFromUsers(users) + if (accountHolder) { + users = users.filter(u => u.userId !== accountHolder.userId) // mark user as unsuccessful response.unsuccessful.push({ - _id: account.budibaseUserId, - email: account.email, + _id: accountHolder.userId, + email: accountHolder.email, reason: "Account holder cannot be deleted", }) } @@ -435,7 +438,7 @@ export class UserDB { // Get users and delete const allDocsResponse = await db.allDocs({ include_docs: true, - keys: userIds, + keys: users.map(u => u.userId), }) const usersToDelete = allDocsResponse.rows.map(user => { return user.doc! diff --git a/packages/backend-core/src/users/users.ts b/packages/backend-core/src/users/users.ts index f4838597b6..0bff428fa9 100644 --- a/packages/backend-core/src/users/users.ts +++ b/packages/backend-core/src/users/users.ts @@ -70,6 +70,17 @@ export async function getAllUserIds() { return response.rows.map(row => row.id) } +export async function getAllUsers(): Promise { + const db = getGlobalDB() + const startKey = `${DocumentType.USER}${SEPARATOR}` + const response = await db.allDocs({ + startkey: startKey, + endkey: `${startKey}${UNICODE_MAX}`, + include_docs: true, + }) + return response.rows.map(row => row.doc) as User[] +} + export async function bulkUpdateGlobalUsers(users: User[]) { const db = getGlobalDB() return (await db.bulkDocs(users)) as BulkDocsResponse diff --git a/packages/backend-core/src/users/utils.ts b/packages/backend-core/src/users/utils.ts index e1e3da181d..91b667ce17 100644 --- a/packages/backend-core/src/users/utils.ts +++ b/packages/backend-core/src/users/utils.ts @@ -1,11 +1,9 @@ -import { CloudAccount, ContextUser, User, UserGroup } from "@budibase/types" +import { ContextUser, User, UserGroup, UserIdentifier } from "@budibase/types" import * as accountSdk from "../accounts" import env from "../environment" -import { getFirstPlatformUser } from "./lookup" +import { getExistingAccounts, getFirstPlatformUser } from "./lookup" import { EmailUnavailableError } from "../errors" -import { getTenantId } from "../context" import { sdk } from "@budibase/shared-core" -import { getAccountByTenantId } from "../accounts" import { BUILTIN_ROLE_IDS } from "../security/roles" import * as context from "../context" @@ -67,21 +65,17 @@ export async function validateUniqueUser(email: string, tenantId: string) { } /** - * For the given user id's, return the account holder if it is in the ids. + * For a list of users, return the account holder if there is an email match. */ -export async function getAccountHolderFromUserIds( - userIds: string[] -): Promise { +export async function getAccountHolderFromUsers( + users: Array +): Promise { if (!env.SELF_HOSTED && !env.DISABLE_ACCOUNT_PORTAL) { - const tenantId = getTenantId() - const account = await getAccountByTenantId(tenantId) - if (!account) { - throw new Error(`Account not found for tenantId=${tenantId}`) - } - - const budibaseUserId = account.budibaseUserId - if (userIds.includes(budibaseUserId)) { - return account - } + const accountMetadata = await getExistingAccounts( + users.map(user => user.email) + ) + return users.find(user => + accountMetadata.map(metadata => metadata.email).includes(user.email) + ) } } diff --git a/packages/builder/src/pages/builder/portal/users/users/[userId].svelte b/packages/builder/src/pages/builder/portal/users/users/[userId].svelte index 458c9a3f79..3547a4876e 100644 --- a/packages/builder/src/pages/builder/portal/users/users/[userId].svelte +++ b/packages/builder/src/pages/builder/portal/users/users/[userId].svelte @@ -206,7 +206,7 @@ if (!user?._id) { $goto("./") } - tenantOwner = await users.tenantOwner($auth.tenantId) + tenantOwner = await users.getAccountHolder() } async function toggleFlags(detail) { diff --git a/packages/builder/src/pages/builder/portal/users/users/index.svelte b/packages/builder/src/pages/builder/portal/users/users/index.svelte index a1d5496ff6..5a7e334c9c 100644 --- a/packages/builder/src/pages/builder/portal/users/users/index.svelte +++ b/packages/builder/src/pages/builder/portal/users/users/index.svelte @@ -280,7 +280,12 @@ } if (ids.length > 0) { - await users.bulkDelete(ids) + await users.bulkDelete( + selectedRows.map(user => ({ + userId: user._id, + email: user.email, + })) + ) } if (selectedInvites.length > 0) { @@ -319,7 +324,7 @@ groupsLoaded = true pendingInvites = await users.getInvites() invitesLoaded = true - tenantOwner = await users.tenantOwner($auth.tenantId) + tenantOwner = await users.getAccountHolder() tenantOwnerLoaded = true } catch (error) { notifications.error("Error fetching user group data") diff --git a/packages/builder/src/stores/portal/users.js b/packages/builder/src/stores/portal/users.js index e9e65c3803..747dc5144d 100644 --- a/packages/builder/src/stores/portal/users.js +++ b/packages/builder/src/stores/portal/users.js @@ -112,8 +112,8 @@ export function createUsersStore() { return await API.getUserCountByApp({ appId }) } - async function bulkDelete(userIds) { - return API.deleteUsers(userIds) + async function bulkDelete(users) { + return API.deleteUsers(users) } async function save(user) { @@ -128,9 +128,8 @@ export function createUsersStore() { return await API.removeAppBuilder({ userId, appId }) } - async function getTenantOwner(tenantId) { - const tenantInfo = await API.getTenantInfo({ tenantId }) - return tenantInfo?.owner + async function getAccountHolder() { + return await API.getAccountHolder() } const getUserRole = user => { @@ -176,7 +175,7 @@ export function createUsersStore() { save: refreshUsage(save), bulkDelete: refreshUsage(bulkDelete), delete: refreshUsage(del), - tenantOwner: getTenantOwner, + getAccountHolder, } } diff --git a/packages/frontend-core/src/api/user.js b/packages/frontend-core/src/api/user.js index a2846c1e7b..45d481183a 100644 --- a/packages/frontend-core/src/api/user.js +++ b/packages/frontend-core/src/api/user.js @@ -122,14 +122,14 @@ export const buildUserEndpoints = API => ({ /** * Deletes multiple users - * @param userIds the ID of the user to delete + * @param users the ID/email pair of the user to delete */ - deleteUsers: async userIds => { + deleteUsers: async users => { const res = await API.post({ url: `/api/global/users/bulk`, body: { delete: { - userIds, + users, }, }, }) @@ -296,9 +296,9 @@ export const buildUserEndpoints = API => ({ }) }, - getTenantInfo: async ({ tenantId }) => { + getAccountHolder: async () => { return await API.get({ - url: `/api/global/tenant/${tenantId}`, + url: `/api/global/users/accountholder`, }) }, }) diff --git a/packages/types/src/api/web/user.ts b/packages/types/src/api/web/user.ts index 471ca86616..e102f136c3 100644 --- a/packages/types/src/api/web/user.ts +++ b/packages/types/src/api/web/user.ts @@ -15,7 +15,10 @@ export interface UserDetails { export interface BulkUserRequest { delete?: { - userIds: string[] + users: Array<{ + userId: string + email: string + }> } create?: { roles?: any[] diff --git a/packages/types/src/documents/global/index.ts b/packages/types/src/documents/global/index.ts index 6784f2638c..b728439dd6 100644 --- a/packages/types/src/documents/global/index.ts +++ b/packages/types/src/documents/global/index.ts @@ -7,4 +7,3 @@ export * from "./schedule" export * from "./templates" export * from "./environmentVariables" export * from "./auditLogs" -export * from "./tenantInfo" diff --git a/packages/types/src/documents/global/tenantInfo.ts b/packages/types/src/documents/global/tenantInfo.ts deleted file mode 100644 index 4c8837cf2a..0000000000 --- a/packages/types/src/documents/global/tenantInfo.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { Hosting } from "../../sdk" -import { Document } from "../document" - -export interface TenantInfo extends Document { - owner: { - email: string - password?: string - ssoId?: string - givenName?: string - familyName?: string - budibaseUserId?: string - } - tenantId: string - hosting: Hosting -} diff --git a/packages/types/src/documents/global/user.ts b/packages/types/src/documents/global/user.ts index 829a171843..1b242886b5 100644 --- a/packages/types/src/documents/global/user.ts +++ b/packages/types/src/documents/global/user.ts @@ -38,6 +38,11 @@ export function isSSOUser(user: User): user is SSOUser { // USER +export interface UserIdentifier { + userId: string + email: string +} + export interface User extends Document { tenantId: string email: string diff --git a/packages/worker/src/api/controllers/global/tenant.ts b/packages/worker/src/api/controllers/global/tenant.ts deleted file mode 100644 index 8b5ae6d528..0000000000 --- a/packages/worker/src/api/controllers/global/tenant.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { tenancy } from "@budibase/backend-core" -import { TenantInfo, Ctx } from "@budibase/types" - -export const save = async (ctx: Ctx) => { - const response = await tenancy.saveTenantInfo(ctx.request.body) - ctx.body = { - _id: response.id, - _rev: response.rev, - } -} - -export const get = async (ctx: Ctx) => { - ctx.body = await tenancy.getTenantInfo(ctx.params.id) -} diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index 921e0324d1..52f8821fab 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -23,9 +23,11 @@ import { SearchUsersRequest, User, UserCtx, + UserIdentifier, } from "@budibase/types" import { accounts, + users, cache, ErrorCode, events, @@ -55,8 +57,8 @@ export const save = async (ctx: UserCtx) => { const requestUser = ctx.request.body // Do not allow the account holder role to be changed - const tenantInfo = await tenancy.getTenantInfo(requestUser.tenantId) - if (tenantInfo?.owner.email === requestUser.email) { + const accountMetadata = await users.getExistingAccounts([requestUser.email]) + if (accountMetadata?.length > 0) { if ( requestUser.admin?.global !== true || requestUser.builder?.global !== true @@ -103,11 +105,14 @@ export const addSsoSupport = async (ctx: Ctx) => { } } -const bulkDelete = async (userIds: string[], currentUserId: string) => { - if (userIds?.indexOf(currentUserId) !== -1) { +const bulkDelete = async ( + users: Array, + currentUserId: string +) => { + if (users.find(u => u.userId === currentUserId)) { throw new Error("Unable to delete self.") } - return await userSdk.db.bulkDelete(userIds) + return await userSdk.db.bulkDelete(users) } const bulkCreate = async (users: User[], groupIds: string[]) => { @@ -130,7 +135,7 @@ export const bulkUpdate = async ( created = await bulkCreate(input.create.users, input.create.groups) } if (input.delete) { - deleted = await bulkDelete(input.delete.userIds, currentUserId) + deleted = await bulkDelete(input.delete.users, currentUserId) } } catch (err: any) { ctx.throw(err.status || 400, err?.message || err) @@ -302,6 +307,23 @@ export const tenantUserLookup = async (ctx: any) => { } } +/** + * This will be paginated to a default of the first 50 users, + * So the account holder may not be found until further pagination has occurred + */ +export const accountHolderLookup = async (ctx: Ctx) => { + const users = await userSdk.core.getAllUsers() + const response = await userSdk.core.getExistingAccounts( + users.map(u => u.email) + ) + const holder = response[0] + if (!holder) { + return + } + holder._id = users.find(u => u.email === holder.email)?._id + ctx.body = holder +} + /* Encapsulate the app user onboarding flows here. */ diff --git a/packages/worker/src/api/index.ts b/packages/worker/src/api/index.ts index db0a80acfd..a7594b713e 100644 --- a/packages/worker/src/api/index.ts +++ b/packages/worker/src/api/index.ts @@ -71,10 +71,6 @@ const PUBLIC_ENDPOINTS = [ route: "/api/global/users/invite", method: "GET", }, - { - route: "/api/global/tenant", - method: "POST", - }, ] const NO_TENANCY_ENDPOINTS = [ @@ -121,11 +117,7 @@ const NO_TENANCY_ENDPOINTS = [ method: "GET", }, { - route: "/api/global/tenant", - method: "POST", - }, - { - route: "/api/global/tenant/:id", + route: "/api/global/users/accountholder", method: "GET", }, ] diff --git a/packages/worker/src/api/routes/global/tenant.ts b/packages/worker/src/api/routes/global/tenant.ts deleted file mode 100644 index f15e374774..0000000000 --- a/packages/worker/src/api/routes/global/tenant.ts +++ /dev/null @@ -1,11 +0,0 @@ -import Router from "@koa/router" -import * as controller from "../../controllers/global/tenant" -import cloudRestricted from "../../../middleware/cloudRestricted" - -const router: Router = new Router() - -router - .post("/api/global/tenant", cloudRestricted, controller.save) - .get("/api/global/tenant/:id", controller.get) - -export default router diff --git a/packages/worker/src/api/routes/global/tests/tenant.spec.ts b/packages/worker/src/api/routes/global/tests/tenant.spec.ts deleted file mode 100644 index 390cfa2af5..0000000000 --- a/packages/worker/src/api/routes/global/tests/tenant.spec.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { Hosting, TenantInfo } from "@budibase/types" -import { TestConfiguration } from "../../../../tests" -import { tenancy as _tenancy } from "@budibase/backend-core" - -const tenancy = jest.mocked(_tenancy) - -describe("/api/global/tenant", () => { - const config = new TestConfiguration() - - beforeAll(async () => { - await config.beforeAll() - }) - - afterAll(async () => { - await config.afterAll() - }) - - beforeEach(() => { - jest.clearAllMocks() - }) - - describe("POST /api/global/tenant", () => { - it("should save the tenantInfo", async () => { - tenancy.saveTenantInfo = jest.fn().mockImplementation(async () => ({ - id: "DOC_ID", - ok: true, - rev: "DOC_REV", - })) - const tenantInfo: TenantInfo = { - owner: { - email: "test@example.com", - password: "PASSWORD123!", - ssoId: "SSO_ID", - givenName: "Jane", - familyName: "Doe", - budibaseUserId: "USER_ID", - }, - tenantId: "tenant123", - hosting: Hosting.CLOUD, - } - const response = await config.api.tenants.saveTenantInfo(tenantInfo) - - expect(_tenancy.saveTenantInfo).toHaveBeenCalledTimes(1) - expect(_tenancy.saveTenantInfo).toHaveBeenCalledWith(tenantInfo) - expect(response.text).toEqual('{"_id":"DOC_ID","_rev":"DOC_REV"}') - }) - }) -}) diff --git a/packages/worker/src/api/routes/global/tests/users.spec.ts b/packages/worker/src/api/routes/global/tests/users.spec.ts index c8e71f7eb4..b6237c7b4b 100644 --- a/packages/worker/src/api/routes/global/tests/users.spec.ts +++ b/packages/worker/src/api/routes/global/tests/users.spec.ts @@ -412,28 +412,6 @@ describe("/api/global/users", () => { expect(events.user.permissionBuilderRemoved).toHaveBeenCalledTimes(1) }) - it("should not be able to update an account holder user to a basic user", async () => { - const accountHolderUser = await config.createUser( - structures.users.adminUser() - ) - jest.clearAllMocks() - tenancy.getTenantInfo = jest.fn().mockImplementation(() => ({ - owner: { - email: accountHolderUser.email, - }, - })) - - accountHolderUser.admin!.global = false - accountHolderUser.builder!.global = false - - await config.api.users.saveUser(accountHolderUser, 400) - - expect(events.user.created).not.toHaveBeenCalled() - expect(events.user.updated).not.toHaveBeenCalled() - expect(events.user.permissionAdminRemoved).not.toHaveBeenCalled() - expect(events.user.permissionBuilderRemoved).not.toHaveBeenCalled() - }) - it("should be able to update an builder user to a basic user", async () => { const user = await config.createUser(structures.users.builderUser()) jest.clearAllMocks() @@ -592,55 +570,21 @@ describe("/api/global/users", () => { describe("POST /api/global/users/bulk (delete)", () => { it("should not be able to bulk delete current user", async () => { - const user = await config.user! + const user = config.user! - const response = await config.api.users.bulkDeleteUsers([user._id!], 400) + const response = await config.api.users.bulkDeleteUsers( + [ + { + userId: user._id!, + email: "test@example.com", + }, + ], + 400 + ) expect(response.message).toBe("Unable to delete self.") expect(events.user.deleted).not.toHaveBeenCalled() }) - - it("should not be able to bulk delete account owner", async () => { - const user = await config.createUser() - const account = structures.accounts.cloudAccount() - account.budibaseUserId = user._id! - accounts.getAccountByTenantId.mockReturnValue(Promise.resolve(account)) - - const response = await config.api.users.bulkDeleteUsers([user._id!]) - - expect(response.deleted?.successful.length).toBe(0) - expect(response.deleted?.unsuccessful.length).toBe(1) - expect(response.deleted?.unsuccessful[0].reason).toBe( - "Account holder cannot be deleted" - ) - expect(response.deleted?.unsuccessful[0]._id).toBe(user._id) - expect(events.user.deleted).not.toHaveBeenCalled() - }) - - it("should be able to bulk delete users", async () => { - const account = structures.accounts.cloudAccount() - accounts.getAccountByTenantId.mockReturnValue(Promise.resolve(account)) - - const builder = structures.users.builderUser() - const admin = structures.users.adminUser() - const user = structures.users.user() - const createdUsers = await config.api.users.bulkCreateUsers([ - builder, - admin, - user, - ]) - - const toDelete = createdUsers.created?.successful.map( - u => u._id! - ) as string[] - const response = await config.api.users.bulkDeleteUsers(toDelete) - - expect(response.deleted?.successful.length).toBe(3) - expect(response.deleted?.unsuccessful.length).toBe(0) - expect(events.user.deleted).toHaveBeenCalledTimes(3) - expect(events.user.permissionAdminRemoved).toHaveBeenCalledTimes(1) - expect(events.user.permissionBuilderRemoved).toHaveBeenCalledTimes(2) - }) }) describe("POST /api/global/users/search", () => { diff --git a/packages/worker/src/api/routes/global/users.ts b/packages/worker/src/api/routes/global/users.ts index d5dfa47923..4078f348ad 100644 --- a/packages/worker/src/api/routes/global/users.ts +++ b/packages/worker/src/api/routes/global/users.ts @@ -136,6 +136,7 @@ router buildAdminInitValidation(), controller.adminUser ) + .get("/api/global/users/accountholder", controller.accountHolderLookup) .get("/api/global/users/tenant/:id", controller.tenantUserLookup) // global endpoint but needs to come at end (blocks other endpoints otherwise) .get("/api/global/users/:id", auth.builderOrAdmin, controller.find) diff --git a/packages/worker/src/api/routes/index.ts b/packages/worker/src/api/routes/index.ts index 2eb4b5cd5d..e6cacf110f 100644 --- a/packages/worker/src/api/routes/index.ts +++ b/packages/worker/src/api/routes/index.ts @@ -1,7 +1,6 @@ import Router from "@koa/router" import { api as pro } from "@budibase/pro" import userRoutes from "./global/users" -import tenantRoutes from "./global/tenant" import configRoutes from "./global/configs" import workspaceRoutes from "./global/workspaces" import templateRoutes from "./global/templates" @@ -41,7 +40,6 @@ export const routes: Router[] = [ accountRoutes, restoreRoutes, eventRoutes, - tenantRoutes, pro.scim, ] diff --git a/packages/worker/src/api/routes/validation/users.ts b/packages/worker/src/api/routes/validation/users.ts index 333435ed29..46c66285fd 100644 --- a/packages/worker/src/api/routes/validation/users.ts +++ b/packages/worker/src/api/routes/validation/users.ts @@ -66,7 +66,14 @@ export const buildUserBulkUserValidation = (isSelf = false) => { users: Joi.array().items(Joi.object(schema).required().unknown(true)), }), delete: Joi.object({ - userIds: Joi.array().items(Joi.string()), + users: Joi.array().items( + Joi.object({ + email: Joi.string(), + userId: Joi.string(), + }) + .required() + .unknown(true) + ), }), } diff --git a/packages/worker/src/tests/api/tenants.ts b/packages/worker/src/tests/api/tenants.ts index c404b8ad58..16f970915a 100644 --- a/packages/worker/src/tests/api/tenants.ts +++ b/packages/worker/src/tests/api/tenants.ts @@ -1,4 +1,3 @@ -import { TenantInfo } from "@budibase/types" import TestConfiguration from "../TestConfiguration" import { TestAPI, TestAPIOpts } from "./base" @@ -15,12 +14,4 @@ export class TenantAPI extends TestAPI { .set(opts?.headers) .expect(opts?.status ? opts.status : 204) } - - saveTenantInfo = (tenantInfo: TenantInfo) => { - return this.request - .post("/api/global/tenant") - .set(this.config.internalAPIHeaders()) - .send(tenantInfo) - .expect(200) - } } diff --git a/packages/worker/src/tests/api/users.ts b/packages/worker/src/tests/api/users.ts index 8ff31f04fc..b54fb45d2c 100644 --- a/packages/worker/src/tests/api/users.ts +++ b/packages/worker/src/tests/api/users.ts @@ -81,8 +81,14 @@ export class UserAPI extends TestAPI { return res.body as BulkUserResponse } - bulkDeleteUsers = async (userIds: string[], status?: number) => { - const body: BulkUserRequest = { delete: { userIds } } + bulkDeleteUsers = async ( + users: Array<{ + userId: string + email: string + }>, + status?: number + ) => { + const body: BulkUserRequest = { delete: { users } } const res = await this.request .post(`/api/global/users/bulk`) .send(body) From 3768cc462ab5ec61f33820735dd9bb8249858d0e Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Mon, 28 Oct 2024 14:55:37 +0000 Subject: [PATCH 30/38] Bump version to 2.33.4 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 8235ee9b60..530c51d20b 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.33.3", + "version": "2.33.4", "npmClient": "yarn", "packages": [ "packages/*", From 40a0167c8929a0d828ea8adf6a6562b87f1f87a4 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 28 Oct 2024 16:34:39 +0100 Subject: [PATCH 31/38] Release feature/automation-branching-ux to QA --- .github/workflows/deploy-qa.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/deploy-qa.yml b/.github/workflows/deploy-qa.yml index 2a30e44def..92c957acbd 100644 --- a/.github/workflows/deploy-qa.yml +++ b/.github/workflows/deploy-qa.yml @@ -3,7 +3,7 @@ name: Deploy QA on: push: branches: - - v3-ui + - feature/automation-branching-ux workflow_dispatch: jobs: From a7fa7368d9c223400cea9f67bd6645ea428739e3 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 29 Oct 2024 10:38:58 +0100 Subject: [PATCH 32/38] Reset the page every time that a filter gets updated --- .../app/[application]/settings/automations/index.svelte | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/builder/src/pages/builder/app/[application]/settings/automations/index.svelte b/packages/builder/src/pages/builder/app/[application]/settings/automations/index.svelte index 6518b6cf58..02b063bca8 100644 --- a/packages/builder/src/pages/builder/app/[application]/settings/automations/index.svelte +++ b/packages/builder/src/pages/builder/app/[application]/settings/automations/index.svelte @@ -38,8 +38,11 @@ let loaded = false $: app = $appsStore.apps.find(app => $appStore.appId?.includes(app.appId)) $: licensePlan = $auth.user?.license?.plan - $: page = $pageInfo.page - $: fetchLogs(automationId, status, page, timeRange) + + // Reset the page every time that a filter gets updated + $: pageInfo.reset(), automationId, status, timeRange + + $: fetchLogs(automationId, status, $pageInfo.page, timeRange) $: isCloud = $admin.cloud $: chainAutomations = app?.automations?.chainAutomations ?? !isCloud const timeOptions = [ From 70cd255ef02dd2a2eea62d24c5b2ee7417c2e57c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 29 Oct 2024 10:39:23 +0100 Subject: [PATCH 33/38] Update pro submodule --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index f6aebba944..ee0de4f489 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit f6aebba94451ce47bba551926e5ad72bd75f71c6 +Subproject commit ee0de4f489e8530d98a4ed2c3f0f66034c40bc33 From b9b4f88e4a256032d21eed6e92239398b4263c1d Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 29 Oct 2024 10:23:32 +0000 Subject: [PATCH 34/38] Fix default values when using multi-option column and supplying empty array. --- packages/server/src/api/routes/tests/row.spec.ts | 13 +++++++++++++ packages/server/src/utilities/rowProcessor/index.ts | 8 +++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 66e931d0ab..42c3560b36 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -763,12 +763,25 @@ describe.each([ expect(row.food).toEqual(["apple", "orange"]) }) + it("creates a new row with a default value when given an empty list", async () => { + const row = await config.api.row.save(table._id!, { food: [] }) + expect(row.food).toEqual(["apple", "orange"]) + }) + it("does not use default value if value specified", async () => { const row = await config.api.row.save(table._id!, { food: ["orange"], }) expect(row.food).toEqual(["orange"]) }) + + it("resets back to its default value when empty", async () => { + let row = await config.api.row.save(table._id!, { + food: ["orange"], + }) + row = await config.api.row.save(table._id!, { ...row, food: [] }) + expect(row.food).toEqual(["apple", "orange"]) + }) }) describe("user column", () => { diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index 0ffa7600aa..d5cbbd2b64 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -20,6 +20,7 @@ import { User, ViewV2, FeatureFlag, + Document, } from "@budibase/types" import { cloneDeep } from "lodash/fp" import { @@ -134,7 +135,12 @@ async function processDefaultValues(table: Table, row: Row) { } for (const [key, schema] of Object.entries(table.schema)) { - if ("default" in schema && schema.default != null && row[key] == null) { + const isEmpty = + row[key] == null || + row[key] === "" || + (Array.isArray(row[key]) && row[key].length === 0) + + if ("default" in schema && schema.default != null && isEmpty) { let processed: string | string[] if (Array.isArray(schema.default)) { processed = schema.default.map(val => processStringSync(val, ctx)) From 0c8590be9c43a4ecdda68bad35746378b9c1529b Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 29 Oct 2024 10:32:49 +0000 Subject: [PATCH 35/38] Fix lint. --- packages/server/src/utilities/rowProcessor/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index d5cbbd2b64..ed74394a85 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -20,7 +20,6 @@ import { User, ViewV2, FeatureFlag, - Document, } from "@budibase/types" import { cloneDeep } from "lodash/fp" import { From 4a91b86278628a5d4ac94b448675c32bf9d60ff9 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 29 Oct 2024 11:55:49 +0100 Subject: [PATCH 36/38] Extract values from store --- .../app/[application]/settings/automations/index.svelte | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/builder/src/pages/builder/app/[application]/settings/automations/index.svelte b/packages/builder/src/pages/builder/app/[application]/settings/automations/index.svelte index 02b063bca8..dd332c92a9 100644 --- a/packages/builder/src/pages/builder/app/[application]/settings/automations/index.svelte +++ b/packages/builder/src/pages/builder/app/[application]/settings/automations/index.svelte @@ -42,7 +42,8 @@ // Reset the page every time that a filter gets updated $: pageInfo.reset(), automationId, status, timeRange - $: fetchLogs(automationId, status, $pageInfo.page, timeRange) + $: page = $pageInfo.page + $: fetchLogs(automationId, status, page, timeRange) $: isCloud = $admin.cloud $: chainAutomations = app?.automations?.chainAutomations ?? !isCloud const timeOptions = [ From bbab4d35c3b208d9fc250918403da78cc69bc0c4 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 29 Oct 2024 11:55:56 +0100 Subject: [PATCH 37/38] Update pro submodule --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index ee0de4f489..a3a06b0aaf 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit ee0de4f489e8530d98a4ed2c3f0f66034c40bc33 +Subproject commit a3a06b0aaff47a341c877fe270ee5b524b99910c From ffdd46ef1d51e7fec0c9b48817574acd95fc4ee0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 29 Oct 2024 12:00:54 +0100 Subject: [PATCH 38/38] Update pro submodule --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index a3a06b0aaf..2ab8536b60 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit a3a06b0aaff47a341c877fe270ee5b524b99910c +Subproject commit 2ab8536b6005576684810d774f1ac22239218546