From 9d8c18337dc8d438da07a90307ceaff9d9a8951b Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 4 Mar 2024 16:42:41 +0000 Subject: [PATCH 01/11] Type role controller. --- packages/backend-core/src/security/roles.ts | 57 +++++++++--------- packages/pro | 2 +- packages/server/src/api/controllers/role.ts | 58 ++++++++++--------- .../server/src/api/controllers/routing.ts | 2 +- .../src/api/routes/tests/application.spec.ts | 11 +++- .../src/api/routes/tests/utilities/index.ts | 7 +-- .../src/tests/utilities/TestConfiguration.ts | 10 ++++ packages/types/src/api/web/index.ts | 1 + packages/types/src/api/web/role.ts | 22 +++++++ 9 files changed, 102 insertions(+), 68 deletions(-) create mode 100644 packages/types/src/api/web/role.ts diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 4f048c0a11..01473ad991 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -84,16 +84,18 @@ export function getBuiltinRoles(): { [key: string]: RoleDoc } { return cloneDeep(BUILTIN_ROLES) } -export const BUILTIN_ROLE_ID_ARRAY = Object.values(BUILTIN_ROLES).map( - role => role._id -) +export function isBuiltin(role: string) { + return getBuiltinRole(role) !== undefined +} -export const BUILTIN_ROLE_NAME_ARRAY = Object.values(BUILTIN_ROLES).map( - role => role.name -) - -export function isBuiltin(role?: string) { - return BUILTIN_ROLE_ID_ARRAY.some(builtin => role?.includes(builtin)) +export function getBuiltinRole(roleId: string): Role | undefined { + const role = Object.values(BUILTIN_ROLES).find(role => + roleId.includes(role._id) + ) + if (!role) { + return undefined + } + return cloneDeep(role) } /** @@ -123,7 +125,7 @@ export function builtinRoleToNumber(id?: string) { /** * Converts any role to a number, but has to be async to get the roles from db. */ -export async function roleToNumber(id?: string) { +export async function roleToNumber(id: string) { if (isBuiltin(id)) { return builtinRoleToNumber(id) } @@ -131,7 +133,7 @@ export async function roleToNumber(id?: string) { defaultPublic: true, })) as RoleDoc[] for (let role of hierarchy) { - if (isBuiltin(role?.inherits)) { + if (role?.inherits && isBuiltin(role.inherits)) { return builtinRoleToNumber(role.inherits) + 1 } } @@ -161,35 +163,28 @@ export function lowerBuiltinRoleID(roleId1?: string, roleId2?: string): string { * @returns The role object, which may contain an "inherits" property. */ export async function getRole( - roleId?: string, + roleId: string, opts?: { defaultPublic?: boolean } -): Promise { - if (!roleId) { - return undefined - } - let role: any = {} +): Promise { // built in roles mostly come from the in-code implementation, // but can be extended by a doc stored about them (e.g. permissions) - if (isBuiltin(roleId)) { - role = cloneDeep( - Object.values(BUILTIN_ROLES).find(role => role._id === roleId) - ) - } else { + let role: RoleDoc | undefined = getBuiltinRole(roleId) + if (!role) { // make sure has the prefix (if it has it then it won't be added) roleId = prefixRoleID(roleId) } try { const db = getAppDB() - const dbRole = await db.get(getDBRoleID(roleId)) - role = Object.assign(role, dbRole) + const dbRole = await db.get(getDBRoleID(roleId)) + role = Object.assign(role || {}, dbRole) // finalise the ID - role._id = getExternalRoleID(role._id, role.version) + role._id = getExternalRoleID(role._id!, role.version) } catch (err) { if (!isBuiltin(roleId) && opts?.defaultPublic) { return cloneDeep(BUILTIN_ROLES.PUBLIC) } // only throw an error if there is no role at all - if (Object.keys(role).length === 0) { + if (!role || Object.keys(role).length === 0) { throw err } } @@ -200,7 +195,7 @@ export async function getRole( * Simple function to get all the roles based on the top level user role ID. */ async function getAllUserRoles( - userRoleId?: string, + userRoleId: string, opts?: { defaultPublic?: boolean } ): Promise { // admins have access to all roles @@ -226,7 +221,7 @@ async function getAllUserRoles( } export async function getUserRoleIdHierarchy( - userRoleId?: string + userRoleId: string ): Promise { const roles = await getUserRoleHierarchy(userRoleId) return roles.map(role => role._id!) @@ -241,7 +236,7 @@ export async function getUserRoleIdHierarchy( * highest level of access and the last being the lowest level. */ export async function getUserRoleHierarchy( - userRoleId?: string, + userRoleId: string, opts?: { defaultPublic?: boolean } ) { // special case, if they don't have a role then they are a public user @@ -265,9 +260,9 @@ export function checkForRoleResourceArray( return rolePerms } -export async function getAllRoleIds(appId?: string) { +export async function getAllRoleIds(appId: string): Promise { const roles = await getAllRoles(appId) - return roles.map(role => role._id) + return roles.map(role => role._id!) } /** diff --git a/packages/pro b/packages/pro index 183b35d3ac..22a278da72 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 183b35d3acd42433dcb2d32bcd89a36abe13afec +Subproject commit 22a278da720d92991dabdcd4cb6c96e7abe29781 diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index ae6b89e6d4..ffc1d74209 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -7,8 +7,14 @@ import { } from "@budibase/backend-core" import { getUserMetadataParams, InternalTables } from "../../db/utils" import { + AccessibleRolesResponse, Database, + DestroyRoleResponse, + FetchRolesResponse, + FindRoleResponse, Role, + SaveRoleRequest, + SaveRoleResponse, UserCtx, UserMetadata, UserRoles, @@ -25,43 +31,35 @@ async function updateRolesOnUserTable( db: Database, roleId: string, updateOption: string, - roleVersion: string | undefined + roleVersion?: string ) { const table = await sdk.tables.getTable(InternalTables.USER_METADATA) - const schema = table.schema - const remove = updateOption === UpdateRolesOptions.REMOVED - let updated = false - for (let prop of Object.keys(schema)) { - if (prop === "roleId") { - updated = true - const constraints = schema[prop].constraints! - const updatedRoleId = - roleVersion === roles.RoleIDVersion.NAME - ? roles.getExternalRoleID(roleId, roleVersion) - : roleId - const indexOfRoleId = constraints.inclusion!.indexOf(updatedRoleId) - if (remove && indexOfRoleId !== -1) { - constraints.inclusion!.splice(indexOfRoleId, 1) - } else if (!remove && indexOfRoleId === -1) { - constraints.inclusion!.push(updatedRoleId) - } - break + const constraints = table.schema.roleId?.constraints + if (constraints) { + const updatedRoleId = + roleVersion === roles.RoleIDVersion.NAME + ? roles.getExternalRoleID(roleId, roleVersion) + : roleId + const indexOfRoleId = constraints.inclusion!.indexOf(updatedRoleId) + const remove = updateOption === UpdateRolesOptions.REMOVED + if (remove && indexOfRoleId !== -1) { + constraints.inclusion!.splice(indexOfRoleId, 1) + } else if (!remove && indexOfRoleId === -1) { + constraints.inclusion!.push(updatedRoleId) } - } - if (updated) { await db.put(table) } } -export async function fetch(ctx: UserCtx) { +export async function fetch(ctx: UserCtx) { ctx.body = await roles.getAllRoles() } -export async function find(ctx: UserCtx) { +export async function find(ctx: UserCtx) { ctx.body = await roles.getRole(ctx.params.roleId) } -export async function save(ctx: UserCtx) { +export async function save(ctx: UserCtx) { const db = context.getAppDB() let { _id, name, inherits, permissionId, version } = ctx.request.body let isCreate = false @@ -109,9 +107,9 @@ export async function save(ctx: UserCtx) { ctx.body = role } -export async function destroy(ctx: UserCtx) { +export async function destroy(ctx: UserCtx) { const db = context.getAppDB() - let roleId = ctx.params.roleId + let roleId = ctx.params.roleId as string if (roles.isBuiltin(roleId)) { ctx.throw(400, "Cannot delete builtin role.") } else { @@ -144,14 +142,18 @@ export async function destroy(ctx: UserCtx) { ctx.status = 200 } -export async function accessible(ctx: UserCtx) { +export async function accessible(ctx: UserCtx) { let roleId = ctx.user?.roleId if (!roleId) { roleId = roles.BUILTIN_ROLE_IDS.PUBLIC } if (ctx.user && sharedSdk.users.isAdminOrBuilder(ctx.user)) { const appId = context.getAppId() - ctx.body = await roles.getAllRoleIds(appId) + if (!appId) { + ctx.body = [] + } else { + ctx.body = await roles.getAllRoleIds(appId) + } } else { ctx.body = await roles.getUserRoleIdHierarchy(roleId!) } diff --git a/packages/server/src/api/controllers/routing.ts b/packages/server/src/api/controllers/routing.ts index 4154c6b597..040cda4dd0 100644 --- a/packages/server/src/api/controllers/routing.ts +++ b/packages/server/src/api/controllers/routing.ts @@ -63,7 +63,7 @@ export async function fetch(ctx: UserCtx) { export async function clientFetch(ctx: UserCtx) { const routing = await getRoutingStructure() let roleId = ctx.user?.role?._id - const roleIds = await roles.getUserRoleIdHierarchy(roleId) + const roleIds = roleId ? await roles.getUserRoleIdHierarchy(roleId) : [] for (let topLevel of Object.values(routing.routes) as any) { for (let subpathKey of Object.keys(topLevel.subpaths)) { let found = false diff --git a/packages/server/src/api/routes/tests/application.spec.ts b/packages/server/src/api/routes/tests/application.spec.ts index 3e4ad693db..5a3be462e8 100644 --- a/packages/server/src/api/routes/tests/application.spec.ts +++ b/packages/server/src/api/routes/tests/application.spec.ts @@ -251,10 +251,15 @@ describe("/applications", () => { describe("permissions", () => { it("should only return apps a user has access to", async () => { - const user = await config.createUser() + const user = await config.createUser({ + builder: { global: false }, + admin: { global: false }, + }) - const apps = await config.api.application.fetch() - expect(apps.length).toBeGreaterThan(0) + await config.withUser(user, async () => { + const apps = await config.api.application.fetch() + expect(apps).toHaveLength(0) + }) }) }) }) diff --git a/packages/server/src/api/routes/tests/utilities/index.ts b/packages/server/src/api/routes/tests/utilities/index.ts index 915ff5d970..dcb8ccd6c0 100644 --- a/packages/server/src/api/routes/tests/utilities/index.ts +++ b/packages/server/src/api/routes/tests/utilities/index.ts @@ -1,5 +1,4 @@ -import TestConfig from "../../../../tests/utilities/TestConfiguration" -import env from "../../../../environment" +import TestConfiguration from "../../../../tests/utilities/TestConfiguration" import supertest from "supertest" export * as structures from "../../../../tests/utilities/structures" @@ -47,10 +46,10 @@ export function delay(ms: number) { } let request: supertest.SuperTest | undefined | null, - config: TestConfig | null + config: TestConfiguration | null export function beforeAll() { - config = new TestConfig() + config = new TestConfiguration() request = config.getRequest() } diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 35ca2982c0..2127e9d1cd 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -299,6 +299,16 @@ export default class TestConfiguration { } } + withUser(user: User, f: () => Promise) { + const oldUser = this.user + this.user = user + try { + return f() + } finally { + this.user = oldUser + } + } + // UTILS _req | void, Res>( diff --git a/packages/types/src/api/web/index.ts b/packages/types/src/api/web/index.ts index 9a688a17a5..8a091afdba 100644 --- a/packages/types/src/api/web/index.ts +++ b/packages/types/src/api/web/index.ts @@ -14,3 +14,4 @@ export * from "./cookies" export * from "./automation" export * from "./layout" export * from "./query" +export * from "./role" diff --git a/packages/types/src/api/web/role.ts b/packages/types/src/api/web/role.ts new file mode 100644 index 0000000000..c37dee60e0 --- /dev/null +++ b/packages/types/src/api/web/role.ts @@ -0,0 +1,22 @@ +import { Role } from "../../documents" + +export interface SaveRoleRequest { + _id?: string + _rev?: string + name: string + inherits: string + permissionId: string + version: string +} + +export interface SaveRoleResponse extends Role {} + +export interface FindRoleResponse extends Role {} + +export type FetchRolesResponse = Role[] + +export interface DestroyRoleResponse { + message: string +} + +export type AccessibleRolesResponse = string[] From fced2f369649410d9e8ea7e4ca381fef3d0afb12 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 5 Mar 2024 09:23:48 +0000 Subject: [PATCH 02/11] Respond to PR feedback. --- packages/backend-core/src/security/roles.ts | 2 +- packages/server/src/api/controllers/role.ts | 27 +++++++++++---------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 01473ad991..213c65e18e 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -184,7 +184,7 @@ export async function getRole( return cloneDeep(BUILTIN_ROLES.PUBLIC) } // only throw an error if there is no role at all - if (!role || Object.keys(role).length === 0) { + if (Object.keys(role || {}).length === 0) { throw err } } diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index ffc1d74209..b3eb61a255 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -35,20 +35,21 @@ async function updateRolesOnUserTable( ) { const table = await sdk.tables.getTable(InternalTables.USER_METADATA) const constraints = table.schema.roleId?.constraints - if (constraints) { - const updatedRoleId = - roleVersion === roles.RoleIDVersion.NAME - ? roles.getExternalRoleID(roleId, roleVersion) - : roleId - const indexOfRoleId = constraints.inclusion!.indexOf(updatedRoleId) - const remove = updateOption === UpdateRolesOptions.REMOVED - if (remove && indexOfRoleId !== -1) { - constraints.inclusion!.splice(indexOfRoleId, 1) - } else if (!remove && indexOfRoleId === -1) { - constraints.inclusion!.push(updatedRoleId) - } - await db.put(table) + if (!constraints) { + return } + const updatedRoleId = + roleVersion === roles.RoleIDVersion.NAME + ? roles.getExternalRoleID(roleId, roleVersion) + : roleId + const indexOfRoleId = constraints.inclusion!.indexOf(updatedRoleId) + const remove = updateOption === UpdateRolesOptions.REMOVED + if (remove && indexOfRoleId !== -1) { + constraints.inclusion!.splice(indexOfRoleId, 1) + } else if (!remove && indexOfRoleId === -1) { + constraints.inclusion!.push(updatedRoleId) + } + await db.put(table) } export async function fetch(ctx: UserCtx) { From 488cfea1f432c1dad91680d71afafeacdee5229b Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 5 Mar 2024 14:40:29 +0000 Subject: [PATCH 03/11] Fix typing. --- packages/backend-core/src/security/roles.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 213c65e18e..01473ad991 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -184,7 +184,7 @@ export async function getRole( return cloneDeep(BUILTIN_ROLES.PUBLIC) } // only throw an error if there is no role at all - if (Object.keys(role || {}).length === 0) { + if (!role || Object.keys(role).length === 0) { throw err } } From a332c058ce8f1fb371c84c7beb2aff44c1590354 Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Tue, 5 Mar 2024 16:19:21 +0000 Subject: [PATCH 04/11] Disabling aliasing on writes (create, update, delete) for MySQL/MS-SQL datasources. --- .../server/src/api/controllers/row/alias.ts | 48 +++++++++++++++---- packages/server/src/integrations/base/sql.ts | 10 ++-- packages/server/src/integrations/index.ts | 11 +++-- packages/server/src/sdk/app/rows/utils.ts | 20 ++++++++ 4 files changed, 74 insertions(+), 15 deletions(-) diff --git a/packages/server/src/api/controllers/row/alias.ts b/packages/server/src/api/controllers/row/alias.ts index 46b090bb97..0adcfaa582 100644 --- a/packages/server/src/api/controllers/row/alias.ts +++ b/packages/server/src/api/controllers/row/alias.ts @@ -1,12 +1,16 @@ import { - QueryJson, - SearchFilters, - Table, - Row, + Datasource, DatasourcePlusQueryResponse, + Operation, + QueryJson, + Row, + SearchFilters, } from "@budibase/types" -import { getDatasourceAndQuery } from "../../../sdk/app/rows/utils" +import { getSQLClient } from "../../../sdk/app/rows/utils" import { cloneDeep } from "lodash" +import sdk from "../../../sdk" +import { makeExternalQuery } from "../../../integrations/base/query" +import { SqlClient } from "../../../integrations/utils" class CharSequence { static alphabet = "abcdefghijklmnopqrstuvwxyz" @@ -43,6 +47,32 @@ export default class AliasTables { this.charSeq = new CharSequence() } + isAliasingEnabled(json: QueryJson, datasource: Datasource) { + const fieldLength = json.resource?.fields?.length + if (!fieldLength || fieldLength <= 0) { + return false + } + const writeOperations = [ + Operation.CREATE, + Operation.UPDATE, + Operation.DELETE, + ] + try { + const sqlClient = getSQLClient(datasource) + const isWrite = writeOperations.includes(json.endpoint.operation) + if ( + isWrite && + (sqlClient === SqlClient.MY_SQL || sqlClient === SqlClient.MS_SQL) + ) { + return false + } + } catch (err) { + // if we can't get an SQL client, we can't alias + return false + } + return true + } + getAlias(tableName: string) { if (this.aliases[tableName]) { return this.aliases[tableName] @@ -111,8 +141,10 @@ export default class AliasTables { } async queryWithAliasing(json: QueryJson): DatasourcePlusQueryResponse { - const fieldLength = json.resource?.fields?.length - const aliasingEnabled = fieldLength && fieldLength > 0 + const datasourceId = json.endpoint.datasourceId + const datasource = await sdk.datasources.get(datasourceId) + + const aliasingEnabled = this.isAliasingEnabled(json, datasource) if (aliasingEnabled) { json = cloneDeep(json) // run through the query json to update anywhere a table may be used @@ -158,7 +190,7 @@ export default class AliasTables { } json.tableAliases = invertedTableAliases } - const response = await getDatasourceAndQuery(json) + const response = await makeExternalQuery(datasource, json) if (Array.isArray(response) && aliasingEnabled) { return this.reverse(response) } else { diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index c8acb606b3..be1883c8ec 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -435,10 +435,12 @@ class InternalBuilder { aliases?: QueryJson["tableAliases"] ): Knex.QueryBuilder { const tableName = endpoint.entityId - const tableAliased = aliases?.[tableName] - ? `${tableName} as ${aliases?.[tableName]}` - : tableName - let query = knex(tableAliased) + const tableAlias = aliases?.[tableName] + let table: string | Record = tableName + if (tableAlias) { + table = { [tableAlias]: tableName } + } + let query = knex(table) if (endpoint.schema) { query = query.withSchema(endpoint.schema) } diff --git a/packages/server/src/integrations/index.ts b/packages/server/src/integrations/index.ts index ee2bb23f23..18c46b9260 100644 --- a/packages/server/src/integrations/index.ts +++ b/packages/server/src/integrations/index.ts @@ -14,13 +14,18 @@ import firebase from "./firebase" import redis from "./redis" import snowflake from "./snowflake" import oracle from "./oracle" -import { SourceName, Integration, PluginType } from "@budibase/types" +import { + SourceName, + Integration, + PluginType, + IntegrationBase, +} from "@budibase/types" import { getDatasourcePlugin } from "../utilities/fileSystem" import env from "../environment" import cloneDeep from "lodash/cloneDeep" import sdk from "../sdk" -const DEFINITIONS: Record = { +const DEFINITIONS: { [key: SourceName]: Integration | undefined } = { [SourceName.POSTGRES]: postgres.schema, [SourceName.DYNAMODB]: dynamodb.schema, [SourceName.MONGODB]: mongodb.schema, @@ -40,7 +45,7 @@ const DEFINITIONS: Record = { [SourceName.BUDIBASE]: undefined, } -const INTEGRATIONS: Record = { +const INTEGRATIONS: { [key: SourceName]: IntegrationBase | undefined } = { [SourceName.POSTGRES]: postgres.integration, [SourceName.DYNAMODB]: dynamodb.integration, [SourceName.MONGODB]: mongodb.integration, diff --git a/packages/server/src/sdk/app/rows/utils.ts b/packages/server/src/sdk/app/rows/utils.ts index a8052462a9..e090045925 100644 --- a/packages/server/src/sdk/app/rows/utils.ts +++ b/packages/server/src/sdk/app/rows/utils.ts @@ -7,11 +7,31 @@ import { Table, TableSchema, DatasourcePlusQueryResponse, + Datasource, + SourceName, } from "@budibase/types" import { makeExternalQuery } from "../../../integrations/base/query" import { Format } from "../../../api/controllers/view/exporters" import sdk from "../.." import { isRelationshipColumn } from "../../../db/utils" +import { SqlClient } from "../../../integrations/utils" + +export function getSQLClient(datasource: Datasource): SqlClient { + if (!datasource.isSQL) { + throw new Error("Cannot get SQL Client for non-SQL datasource") + } + switch (datasource.source) { + case SourceName.POSTGRES: + return SqlClient.POSTGRES + case SourceName.MYSQL: + return SqlClient.MY_SQL + case SourceName.ORACLE: + return SqlClient.ORACLE + case SourceName.SQL_SERVER: + return SqlClient.MS_SQL + } + throw new Error("Unable to find a valid SQL client") +} export async function getDatasourceAndQuery( json: QueryJson From 0520c0c54083eed0900f12017dfcee2b50257bec Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Tue, 5 Mar 2024 17:27:35 +0000 Subject: [PATCH 05/11] Adding tests to confirm when aliasing should be used. --- .../server/src/api/controllers/row/alias.ts | 29 +++----- .../src/integrations/tests/sqlAlias.spec.ts | 74 ++++++++++++++++++- 2 files changed, 84 insertions(+), 19 deletions(-) diff --git a/packages/server/src/api/controllers/row/alias.ts b/packages/server/src/api/controllers/row/alias.ts index 0adcfaa582..28eaa95bf8 100644 --- a/packages/server/src/api/controllers/row/alias.ts +++ b/packages/server/src/api/controllers/row/alias.ts @@ -1,17 +1,17 @@ -import { - Datasource, - DatasourcePlusQueryResponse, - Operation, - QueryJson, - Row, - SearchFilters, -} from "@budibase/types" +import { Datasource, DatasourcePlusQueryResponse, Operation, QueryJson, Row, SearchFilters } from "@budibase/types" import { getSQLClient } from "../../../sdk/app/rows/utils" import { cloneDeep } from "lodash" import sdk from "../../../sdk" import { makeExternalQuery } from "../../../integrations/base/query" import { SqlClient } from "../../../integrations/utils" +const WRITE_OPERATIONS: Operation[] = [ + Operation.CREATE, + Operation.UPDATE, + Operation.DELETE, +] +const DISABLED_WRITE_CLIENTS: SqlClient[] = [SqlClient.MY_SQL, SqlClient.MS_SQL, SqlClient.ORACLE] + class CharSequence { static alphabet = "abcdefghijklmnopqrstuvwxyz" counters: number[] @@ -52,18 +52,11 @@ export default class AliasTables { if (!fieldLength || fieldLength <= 0) { return false } - const writeOperations = [ - Operation.CREATE, - Operation.UPDATE, - Operation.DELETE, - ] try { const sqlClient = getSQLClient(datasource) - const isWrite = writeOperations.includes(json.endpoint.operation) - if ( - isWrite && - (sqlClient === SqlClient.MY_SQL || sqlClient === SqlClient.MS_SQL) - ) { + const isWrite = WRITE_OPERATIONS.includes(json.endpoint.operation) + const isDisabledClient = DISABLED_WRITE_CLIENTS.includes(sqlClient) + if (isWrite && isDisabledClient) { return false } } catch (err) { diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index fe9798aaa0..70dda8c335 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -1,4 +1,4 @@ -import { QueryJson } from "@budibase/types" +import { Datasource, Operation, QueryJson, SourceName } from "@budibase/types" import { join } from "path" import Sql from "../base/sql" import { SqlClient } from "../utils" @@ -198,6 +198,78 @@ describe("Captures of real examples", () => { }) }) + describe("check aliasing is disabled/enabled", () => { + const tables = ["tableA", "tableB"] + + function getDatasource(source: SourceName): Datasource { + return { + source, + type: "datasource", + isSQL: true, + } + } + + function getQuery(op: Operation, fields: string[] = ["a"]): QueryJson { + return { + endpoint: { datasourceId: "", entityId: "", operation: op }, + resource: { + fields, + } + } + } + + it("should check for Postgres aliased status", () => { + const aliasing = new AliasTables(tables) + const datasource = getDatasource(SourceName.POSTGRES) + expect(aliasing.isAliasingEnabled(getQuery(Operation.CREATE), datasource)).toEqual(true) + expect(aliasing.isAliasingEnabled(getQuery(Operation.READ), datasource)).toEqual(true) + expect(aliasing.isAliasingEnabled(getQuery(Operation.UPDATE), datasource)).toEqual(true) + expect(aliasing.isAliasingEnabled(getQuery(Operation.DELETE), datasource)).toEqual(true) + }) + + it("should check for MS-SQL aliased status", () => { + const aliasing = new AliasTables(tables) + const datasource = getDatasource(SourceName.SQL_SERVER) + expect(aliasing.isAliasingEnabled(getQuery(Operation.CREATE), datasource)).toEqual(false) + expect(aliasing.isAliasingEnabled(getQuery(Operation.READ), datasource)).toEqual(true) + expect(aliasing.isAliasingEnabled(getQuery(Operation.UPDATE), datasource)).toEqual(false) + expect(aliasing.isAliasingEnabled(getQuery(Operation.DELETE), datasource)).toEqual(false) + }) + + it("should check for MySQL aliased status", () => { + const aliasing = new AliasTables(tables) + const datasource = getDatasource(SourceName.MYSQL) + expect(aliasing.isAliasingEnabled(getQuery(Operation.CREATE), datasource)).toEqual(false) + expect(aliasing.isAliasingEnabled(getQuery(Operation.READ), datasource)).toEqual(true) + expect(aliasing.isAliasingEnabled(getQuery(Operation.UPDATE), datasource)).toEqual(false) + expect(aliasing.isAliasingEnabled(getQuery(Operation.DELETE), datasource)).toEqual(false) + }) + + it("should check for Oracle aliased status", () => { + const aliasing = new AliasTables(tables) + const datasource = getDatasource(SourceName.ORACLE) + expect(aliasing.isAliasingEnabled(getQuery(Operation.CREATE), datasource)).toEqual(false) + expect(aliasing.isAliasingEnabled(getQuery(Operation.READ), datasource)).toEqual(true) + expect(aliasing.isAliasingEnabled(getQuery(Operation.UPDATE), datasource)).toEqual(false) + expect(aliasing.isAliasingEnabled(getQuery(Operation.DELETE), datasource)).toEqual(false) + }) + + it("should disable aliasing for non-SQL datasources", () => { + const aliasing = new AliasTables(tables) + expect(aliasing.isAliasingEnabled(getQuery(Operation.READ), { + source: SourceName.GOOGLE_SHEETS, + type: "datasource", + isSQL: false, + })) + }) + + it("should disable when no fields", () => { + const aliasing = new AliasTables(tables) + const datasource = getDatasource(SourceName.POSTGRES) + expect(aliasing.isAliasingEnabled(getQuery(Operation.READ, []), datasource)).toEqual(false) + }) + }) + describe("check some edge cases", () => { const tableNames = ["hello", "world"] From edda776b14c09fe97325ded56eebca8970a1f537 Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Tue, 5 Mar 2024 17:42:44 +0000 Subject: [PATCH 06/11] PR comments. --- packages/server/src/sdk/app/rows/utils.ts | 40 +++++++++++++++-------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/packages/server/src/sdk/app/rows/utils.ts b/packages/server/src/sdk/app/rows/utils.ts index e090045925..6e3e25364e 100644 --- a/packages/server/src/sdk/app/rows/utils.ts +++ b/packages/server/src/sdk/app/rows/utils.ts @@ -1,14 +1,14 @@ import cloneDeep from "lodash/cloneDeep" import validateJs from "validate.js" import { + Datasource, + DatasourcePlusQueryResponse, FieldType, QueryJson, Row, + SourceName, Table, TableSchema, - DatasourcePlusQueryResponse, - Datasource, - SourceName, } from "@budibase/types" import { makeExternalQuery } from "../../../integrations/base/query" import { Format } from "../../../api/controllers/view/exporters" @@ -16,21 +16,35 @@ import sdk from "../.." import { isRelationshipColumn } from "../../../db/utils" import { SqlClient } from "../../../integrations/utils" +const SQL_CLIENT_SOURCE_MAP: Record = { + [SourceName.POSTGRES]: SqlClient.POSTGRES, + [SourceName.MYSQL]: SqlClient.MY_SQL, + [SourceName.SQL_SERVER]: SqlClient.MS_SQL, + [SourceName.ORACLE]: SqlClient.ORACLE, + [SourceName.DYNAMODB]: undefined, + [SourceName.MONGODB]: undefined, + [SourceName.ELASTICSEARCH]: undefined, + [SourceName.COUCHDB]: undefined, + [SourceName.S3]: undefined, + [SourceName.AIRTABLE]: undefined, + [SourceName.ARANGODB]: undefined, + [SourceName.REST]: undefined, + [SourceName.FIRESTORE]: undefined, + [SourceName.GOOGLE_SHEETS]: undefined, + [SourceName.REDIS]: undefined, + [SourceName.SNOWFLAKE]: undefined, + [SourceName.BUDIBASE]: undefined, +} + export function getSQLClient(datasource: Datasource): SqlClient { if (!datasource.isSQL) { throw new Error("Cannot get SQL Client for non-SQL datasource") } - switch (datasource.source) { - case SourceName.POSTGRES: - return SqlClient.POSTGRES - case SourceName.MYSQL: - return SqlClient.MY_SQL - case SourceName.ORACLE: - return SqlClient.ORACLE - case SourceName.SQL_SERVER: - return SqlClient.MS_SQL + const lookup = SQL_CLIENT_SOURCE_MAP[datasource.source] + if (lookup) { + return lookup } - throw new Error("Unable to find a valid SQL client") + throw new Error("Unable to determine client for SQL datasource") } export async function getDatasourceAndQuery( From b72edc21ecde8fb9564520d94c9a6046430886d5 Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Tue, 5 Mar 2024 17:46:09 +0000 Subject: [PATCH 07/11] Linting. --- .../server/src/api/controllers/row/alias.ts | 15 +++- .../src/integrations/tests/sqlAlias.spec.ts | 82 +++++++++++++------ 2 files changed, 72 insertions(+), 25 deletions(-) diff --git a/packages/server/src/api/controllers/row/alias.ts b/packages/server/src/api/controllers/row/alias.ts index 28eaa95bf8..1d586c54fd 100644 --- a/packages/server/src/api/controllers/row/alias.ts +++ b/packages/server/src/api/controllers/row/alias.ts @@ -1,4 +1,11 @@ -import { Datasource, DatasourcePlusQueryResponse, Operation, QueryJson, Row, SearchFilters } from "@budibase/types" +import { + Datasource, + DatasourcePlusQueryResponse, + Operation, + QueryJson, + Row, + SearchFilters, +} from "@budibase/types" import { getSQLClient } from "../../../sdk/app/rows/utils" import { cloneDeep } from "lodash" import sdk from "../../../sdk" @@ -10,7 +17,11 @@ const WRITE_OPERATIONS: Operation[] = [ Operation.UPDATE, Operation.DELETE, ] -const DISABLED_WRITE_CLIENTS: SqlClient[] = [SqlClient.MY_SQL, SqlClient.MS_SQL, SqlClient.ORACLE] +const DISABLED_WRITE_CLIENTS: SqlClient[] = [ + SqlClient.MY_SQL, + SqlClient.MS_SQL, + SqlClient.ORACLE, +] class CharSequence { static alphabet = "abcdefghijklmnopqrstuvwxyz" diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index 70dda8c335..dd82dadac0 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -214,59 +214,95 @@ describe("Captures of real examples", () => { endpoint: { datasourceId: "", entityId: "", operation: op }, resource: { fields, - } + }, } } it("should check for Postgres aliased status", () => { const aliasing = new AliasTables(tables) const datasource = getDatasource(SourceName.POSTGRES) - expect(aliasing.isAliasingEnabled(getQuery(Operation.CREATE), datasource)).toEqual(true) - expect(aliasing.isAliasingEnabled(getQuery(Operation.READ), datasource)).toEqual(true) - expect(aliasing.isAliasingEnabled(getQuery(Operation.UPDATE), datasource)).toEqual(true) - expect(aliasing.isAliasingEnabled(getQuery(Operation.DELETE), datasource)).toEqual(true) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.CREATE), datasource) + ).toEqual(true) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.READ), datasource) + ).toEqual(true) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.UPDATE), datasource) + ).toEqual(true) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.DELETE), datasource) + ).toEqual(true) }) it("should check for MS-SQL aliased status", () => { const aliasing = new AliasTables(tables) const datasource = getDatasource(SourceName.SQL_SERVER) - expect(aliasing.isAliasingEnabled(getQuery(Operation.CREATE), datasource)).toEqual(false) - expect(aliasing.isAliasingEnabled(getQuery(Operation.READ), datasource)).toEqual(true) - expect(aliasing.isAliasingEnabled(getQuery(Operation.UPDATE), datasource)).toEqual(false) - expect(aliasing.isAliasingEnabled(getQuery(Operation.DELETE), datasource)).toEqual(false) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.CREATE), datasource) + ).toEqual(false) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.READ), datasource) + ).toEqual(true) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.UPDATE), datasource) + ).toEqual(false) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.DELETE), datasource) + ).toEqual(false) }) it("should check for MySQL aliased status", () => { const aliasing = new AliasTables(tables) const datasource = getDatasource(SourceName.MYSQL) - expect(aliasing.isAliasingEnabled(getQuery(Operation.CREATE), datasource)).toEqual(false) - expect(aliasing.isAliasingEnabled(getQuery(Operation.READ), datasource)).toEqual(true) - expect(aliasing.isAliasingEnabled(getQuery(Operation.UPDATE), datasource)).toEqual(false) - expect(aliasing.isAliasingEnabled(getQuery(Operation.DELETE), datasource)).toEqual(false) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.CREATE), datasource) + ).toEqual(false) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.READ), datasource) + ).toEqual(true) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.UPDATE), datasource) + ).toEqual(false) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.DELETE), datasource) + ).toEqual(false) }) it("should check for Oracle aliased status", () => { const aliasing = new AliasTables(tables) const datasource = getDatasource(SourceName.ORACLE) - expect(aliasing.isAliasingEnabled(getQuery(Operation.CREATE), datasource)).toEqual(false) - expect(aliasing.isAliasingEnabled(getQuery(Operation.READ), datasource)).toEqual(true) - expect(aliasing.isAliasingEnabled(getQuery(Operation.UPDATE), datasource)).toEqual(false) - expect(aliasing.isAliasingEnabled(getQuery(Operation.DELETE), datasource)).toEqual(false) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.CREATE), datasource) + ).toEqual(false) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.READ), datasource) + ).toEqual(true) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.UPDATE), datasource) + ).toEqual(false) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.DELETE), datasource) + ).toEqual(false) }) it("should disable aliasing for non-SQL datasources", () => { const aliasing = new AliasTables(tables) - expect(aliasing.isAliasingEnabled(getQuery(Operation.READ), { - source: SourceName.GOOGLE_SHEETS, - type: "datasource", - isSQL: false, - })) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.READ), { + source: SourceName.GOOGLE_SHEETS, + type: "datasource", + isSQL: false, + }) + ) }) it("should disable when no fields", () => { const aliasing = new AliasTables(tables) const datasource = getDatasource(SourceName.POSTGRES) - expect(aliasing.isAliasingEnabled(getQuery(Operation.READ, []), datasource)).toEqual(false) + expect( + aliasing.isAliasingEnabled(getQuery(Operation.READ, []), datasource) + ).toEqual(false) }) }) From 1918ec6c68f6979d41852061cf8437a7599f9511 Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Tue, 5 Mar 2024 18:00:15 +0000 Subject: [PATCH 08/11] Reverting type changes. --- packages/server/src/integrations/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/integrations/index.ts b/packages/server/src/integrations/index.ts index 18c46b9260..92067d1918 100644 --- a/packages/server/src/integrations/index.ts +++ b/packages/server/src/integrations/index.ts @@ -25,7 +25,7 @@ import env from "../environment" import cloneDeep from "lodash/cloneDeep" import sdk from "../sdk" -const DEFINITIONS: { [key: SourceName]: Integration | undefined } = { +const DEFINITIONS: Record = { [SourceName.POSTGRES]: postgres.schema, [SourceName.DYNAMODB]: dynamodb.schema, [SourceName.MONGODB]: mongodb.schema, @@ -45,7 +45,7 @@ const DEFINITIONS: { [key: SourceName]: Integration | undefined } = { [SourceName.BUDIBASE]: undefined, } -const INTEGRATIONS: { [key: SourceName]: IntegrationBase | undefined } = { +const INTEGRATIONS: Record = { [SourceName.POSTGRES]: postgres.integration, [SourceName.DYNAMODB]: dynamodb.integration, [SourceName.MONGODB]: mongodb.integration, From b58b0d3b40320ae551b3e8c82028b18ce236f8d5 Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Tue, 5 Mar 2024 18:15:19 +0000 Subject: [PATCH 09/11] Fixing integration base types. --- packages/server/src/integrations/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/server/src/integrations/index.ts b/packages/server/src/integrations/index.ts index 92067d1918..747a717278 100644 --- a/packages/server/src/integrations/index.ts +++ b/packages/server/src/integrations/index.ts @@ -45,7 +45,9 @@ const DEFINITIONS: Record = { [SourceName.BUDIBASE]: undefined, } -const INTEGRATIONS: Record = { +type IntegrationBaseConstructor = new (...args: any[]) => IntegrationBase + +const INTEGRATIONS: Record = { [SourceName.POSTGRES]: postgres.integration, [SourceName.DYNAMODB]: dynamodb.integration, [SourceName.MONGODB]: mongodb.integration, From de56324a4b24fd41414369435654c4833fc4dca7 Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Tue, 5 Mar 2024 18:16:27 +0000 Subject: [PATCH 10/11] Linting --- packages/server/src/integrations/index.ts | 39 ++++++++++++----------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/packages/server/src/integrations/index.ts b/packages/server/src/integrations/index.ts index 747a717278..8cbc29251b 100644 --- a/packages/server/src/integrations/index.ts +++ b/packages/server/src/integrations/index.ts @@ -47,25 +47,26 @@ const DEFINITIONS: Record = { type IntegrationBaseConstructor = new (...args: any[]) => IntegrationBase -const INTEGRATIONS: Record = { - [SourceName.POSTGRES]: postgres.integration, - [SourceName.DYNAMODB]: dynamodb.integration, - [SourceName.MONGODB]: mongodb.integration, - [SourceName.ELASTICSEARCH]: elasticsearch.integration, - [SourceName.COUCHDB]: couchdb.integration, - [SourceName.SQL_SERVER]: sqlServer.integration, - [SourceName.S3]: s3.integration, - [SourceName.AIRTABLE]: airtable.integration, - [SourceName.MYSQL]: mysql.integration, - [SourceName.ARANGODB]: arangodb.integration, - [SourceName.REST]: rest.integration, - [SourceName.FIRESTORE]: firebase.integration, - [SourceName.GOOGLE_SHEETS]: googlesheets.integration, - [SourceName.REDIS]: redis.integration, - [SourceName.SNOWFLAKE]: snowflake.integration, - [SourceName.ORACLE]: undefined, - [SourceName.BUDIBASE]: undefined, -} +const INTEGRATIONS: Record = + { + [SourceName.POSTGRES]: postgres.integration, + [SourceName.DYNAMODB]: dynamodb.integration, + [SourceName.MONGODB]: mongodb.integration, + [SourceName.ELASTICSEARCH]: elasticsearch.integration, + [SourceName.COUCHDB]: couchdb.integration, + [SourceName.SQL_SERVER]: sqlServer.integration, + [SourceName.S3]: s3.integration, + [SourceName.AIRTABLE]: airtable.integration, + [SourceName.MYSQL]: mysql.integration, + [SourceName.ARANGODB]: arangodb.integration, + [SourceName.REST]: rest.integration, + [SourceName.FIRESTORE]: firebase.integration, + [SourceName.GOOGLE_SHEETS]: googlesheets.integration, + [SourceName.REDIS]: redis.integration, + [SourceName.SNOWFLAKE]: snowflake.integration, + [SourceName.ORACLE]: undefined, + [SourceName.BUDIBASE]: undefined, + } // optionally add oracle integration if the oracle binary can be installed if ( From bed813da77c2ef5c0eb7a4bdc290f21ae2fd4e4c Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 5 Mar 2024 18:29:11 +0000 Subject: [PATCH 11/11] Bump version to 2.21.3 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 6fb032ac77..a77a16a24e 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.21.2", + "version": "2.21.3", "npmClient": "yarn", "packages": [ "packages/*",