From 9b3ab049a4d46200ed357dbfc7d4282c2686334e Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 8 Oct 2024 11:50:16 +0100 Subject: [PATCH 01/31] wip --- .../src/api/routes/tests/viewV2.spec.ts | 30 +++++++++++++++++++ packages/server/src/sdk/app/views/index.ts | 3 +- packages/types/src/documents/app/row.ts | 20 +++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 5dabae6a03..5b25b48656 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -26,6 +26,8 @@ import { BBReferenceFieldSubType, NumericCalculationFieldMetadata, ViewV2Schema, + canGroupBy, + ViewV2Type, } from "@budibase/types" import { generator, mocks } from "@budibase/backend-core/tests" import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" @@ -721,6 +723,34 @@ describe.each([ }, }) }) + + it.only("cannot use complex types as group-by fields", async () => { + const complexTypes = Object.values(FieldType).filter( + type => !canGroupBy(type) + ) + for (const type of complexTypes) { + const field = { name: "field", type } as FieldSchema + const table = await config.api.table.save( + saveTableRequest({ schema: { field } }) + ) + await config.api.viewV2.create( + { + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + field: { visible: true }, + }, + }, + { + status: 400, + body: { + message: "", + }, + } + ) + } + }) }) describe("update", () => { diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 5445851d88..76b2f6e347 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -1,6 +1,7 @@ import { CalculationType, FieldType, + isNumeric, PermissionLevel, RelationSchemaField, RenameColumn, @@ -103,7 +104,7 @@ async function guardCalculationViewSchema( ) } - if (!isCount && !helpers.schema.isNumeric(targetSchema)) { + if (!isCount && !isNumeric(targetSchema.type)) { throw new HTTPError( `Calculation field "${name}" references field "${schema.field}" which is not a numeric field`, 400 diff --git a/packages/types/src/documents/app/row.ts b/packages/types/src/documents/app/row.ts index 42440b2988..673d481e3c 100644 --- a/packages/types/src/documents/app/row.ts +++ b/packages/types/src/documents/app/row.ts @@ -127,6 +127,26 @@ export const JsonTypes = [ FieldType.ARRAY, ] +export const NumericTypes = [FieldType.NUMBER, FieldType.BIGINT] + +export function isNumeric(type: FieldType) { + return NumericTypes.includes(type) +} + +export const GroupByTypes = [ + FieldType.STRING, + FieldType.LONGFORM, + FieldType.OPTIONS, + FieldType.NUMBER, + FieldType.BOOLEAN, + FieldType.DATETIME, + FieldType.BIGINT, +] + +export function canGroupBy(type: FieldType) { + return GroupByTypes.includes(type) +} + export interface RowAttachment { size: number name: string From 5c3adbed270a91fade0798024c18e0f9bbc19dd8 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 8 Oct 2024 15:34:34 +0100 Subject: [PATCH 02/31] Validate you can't group by complex fields. --- .../src/api/routes/tests/viewV2.spec.ts | 54 +++++++++---------- packages/server/src/sdk/app/views/index.ts | 8 +++ 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index fa8b005ae9..c7c040717a 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -25,8 +25,8 @@ import { BBReferenceFieldSubType, NumericCalculationFieldMetadata, ViewV2Schema, - canGroupBy, ViewV2Type, + JsonTypes, } from "@budibase/types" import { generator, mocks } from "@budibase/backend-core/tests" import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" @@ -738,33 +738,33 @@ describe.each([ }) }) - it.only("cannot use complex types as group-by fields", async () => { - const complexTypes = Object.values(FieldType).filter( - type => !canGroupBy(type) - ) - for (const type of complexTypes) { - const field = { name: "field", type } as FieldSchema - const table = await config.api.table.save( - saveTableRequest({ schema: { field } }) - ) - await config.api.viewV2.create( - { - tableId: table._id!, - name: generator.guid(), - type: ViewV2Type.CALCULATION, - schema: { - field: { visible: true }, + // We don't allow the creation of tables with most JsonTypes when using + // external datasources. + isInternal && + it("cannot use complex types as group-by fields", async () => { + for (const type of JsonTypes) { + const field = { name: "field", type } as FieldSchema + const table = await config.api.table.save( + saveTableRequest({ schema: { field } }) + ) + await config.api.viewV2.create( + { + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + field: { visible: true }, + }, }, - }, - { - status: 400, - body: { - message: "", - }, - } - ) - } - }) + { + status: 400, + body: { + message: `Grouping by fields of type "${type}" is not supported`, + }, + } + ) + } + }) }) describe("update", () => { diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 1d0e633332..44f6beedb1 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -1,5 +1,6 @@ import { CalculationType, + canGroupBy, FieldType, isNumeric, PermissionLevel, @@ -121,6 +122,13 @@ async function guardCalculationViewSchema( 400 ) } + + if (!canGroupBy(targetSchema.type)) { + throw new HTTPError( + `Grouping by fields of type "${targetSchema.type}" is not supported`, + 400 + ) + } } } From 5dc85230d9ee8f373bc11cc484592257c8f95206 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 8 Oct 2024 17:04:45 +0100 Subject: [PATCH 03/31] Add a test to make sure 'shadowing' a field works as expected in view calculations. --- .../src/api/routes/tests/viewV2.spec.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index c7c040717a..58176cb597 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -738,6 +738,41 @@ describe.each([ }) }) + !isLucene && + it("does not get confused when a calculation field shadows a basic one", async () => { + const table = await config.api.table.save( + saveTableRequest({ + schema: { + age: { + name: "age", + type: FieldType.NUMBER, + }, + }, + }) + ) + + await config.api.row.bulkImport(table._id!, { + rows: [{ age: 1 }, { age: 2 }, { age: 3 }], + }) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + age: { + visible: true, + calculationType: CalculationType.SUM, + field: "age", + }, + }, + }) + + const { rows } = await config.api.row.search(view.id) + expect(rows).toHaveLength(1) + expect(rows[0].age).toEqual(6) + }) + // We don't allow the creation of tables with most JsonTypes when using // external datasources. isInternal && From 0182d4a09f8b4657dae2731779fc595fdeea5567 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 8 Oct 2024 17:55:42 +0100 Subject: [PATCH 04/31] Make sure calculation views cannot be used to write or modify rows. --- .../src/api/controllers/row/external.ts | 6 ++ .../src/api/controllers/row/internal.ts | 7 +++ .../src/api/routes/tests/viewV2.spec.ts | 58 +++++++++++++++++++ packages/server/src/sdk/app/rows/external.ts | 5 ++ packages/server/src/sdk/app/rows/internal.ts | 7 ++- 5 files changed, 82 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/external.ts b/packages/server/src/api/controllers/row/external.ts index 18a9be5087..4fcd612ec3 100644 --- a/packages/server/src/api/controllers/row/external.ts +++ b/packages/server/src/api/controllers/row/external.ts @@ -27,6 +27,7 @@ import { } from "../../../utilities/rowProcessor" import { cloneDeep } from "lodash" import { generateIdForRow } from "./utils" +import { helpers } from "@budibase/shared-core" export async function handleRequest( operation: T, @@ -42,6 +43,11 @@ export async function handleRequest( export async function patch(ctx: UserCtx) { const source = await utils.getSource(ctx) + + if (sdk.views.isView(source) && helpers.views.isCalculationView(source)) { + ctx.throw(400, "Cannot update rows through a calculation view") + } + const table = await utils.getTableFromSource(source) const { _id, ...rowData } = ctx.request.body diff --git a/packages/server/src/api/controllers/row/internal.ts b/packages/server/src/api/controllers/row/internal.ts index bb9de6ce52..53b24d517c 100644 --- a/packages/server/src/api/controllers/row/internal.ts +++ b/packages/server/src/api/controllers/row/internal.ts @@ -22,13 +22,20 @@ import sdk from "../../../sdk" import { getLinkedTableIDs } from "../../../db/linkedRows/linkUtils" import { flatten } from "lodash" import { findRow } from "../../../sdk/app/rows/internal" +import { helpers } from "@budibase/shared-core" export async function patch(ctx: UserCtx) { const { tableId } = utils.getSourceId(ctx) const source = await utils.getSource(ctx) + + if (sdk.views.isView(source) && helpers.views.isCalculationView(source)) { + ctx.throw(400, "Cannot update rows through a calculation view") + } + const table = sdk.views.isView(source) ? await sdk.views.getTable(source.id) : source + const inputs = ctx.request.body const isUserTable = tableId === InternalTables.USER_METADATA let oldRow diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 58176cb597..195481ae10 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1978,6 +1978,30 @@ describe.each([ expect(newRow.one).toBeUndefined() expect(newRow.two).toEqual("bar") }) + + it("should not be possible to create a row in a calculation view", async () => { + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + id: { visible: true }, + one: { visible: true }, + }, + }) + + await config.api.row.save( + view.id, + { one: "foo" }, + { + status: 400, + body: { + message: "Cannot insert rows through a calculation view", + status: 400, + }, + } + ) + }) }) describe("patch", () => { @@ -2042,6 +2066,40 @@ describe.each([ expect(row.one).toEqual("foo") expect(row.two).toEqual("newBar") }) + + it("should not be possible to modify a row in a calculation view", async () => { + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + id: { visible: true }, + one: { visible: true }, + }, + }) + + const newRow = await config.api.row.save(table._id!, { + one: "foo", + two: "bar", + }) + + await config.api.row.patch( + view.id, + { + tableId: table._id!, + _id: newRow._id!, + _rev: newRow._rev!, + one: "newFoo", + two: "newBar", + }, + { + status: 400, + body: { + message: "Cannot update rows through a calculation view", + }, + } + ) + }) }) describe("destroy", () => { diff --git a/packages/server/src/sdk/app/rows/external.ts b/packages/server/src/sdk/app/rows/external.ts index 060ef3738a..f7cc38bf0a 100644 --- a/packages/server/src/sdk/app/rows/external.ts +++ b/packages/server/src/sdk/app/rows/external.ts @@ -15,6 +15,7 @@ import { } from "../../../utilities/rowProcessor" import cloneDeep from "lodash/fp/cloneDeep" import { tryExtractingTableAndViewId } from "./utils" +import { helpers } from "@budibase/shared-core" export async function getRow( sourceId: string | Table | ViewV2, @@ -54,6 +55,10 @@ export async function save( source = await sdk.tables.getTable(tableId) } + if (sdk.views.isView(source) && helpers.views.isCalculationView(source)) { + throw new HTTPError("Cannot insert rows through a calculation view", 400) + } + const row = await inputProcessing(userId, cloneDeep(source), inputs) const validateResult = await sdk.rows.utils.validate({ diff --git a/packages/server/src/sdk/app/rows/internal.ts b/packages/server/src/sdk/app/rows/internal.ts index 9306609132..bd03b77f94 100644 --- a/packages/server/src/sdk/app/rows/internal.ts +++ b/packages/server/src/sdk/app/rows/internal.ts @@ -1,4 +1,4 @@ -import { context, db } from "@budibase/backend-core" +import { context, db, HTTPError } from "@budibase/backend-core" import { Row, Table, ViewV2 } from "@budibase/types" import sdk from "../../../sdk" import { finaliseRow } from "../../../api/controllers/row/staticFormula" @@ -10,6 +10,7 @@ import * as linkRows from "../../../db/linkedRows" import { InternalTables } from "../../../db/utils" import { getFullUser } from "../../../utilities/users" import { getSource, tryExtractingTableAndViewId } from "./utils" +import { helpers } from "@budibase/shared-core" export async function save( tableOrViewId: string, @@ -29,6 +30,10 @@ export async function save( table = source } + if (sdk.views.isView(source) && helpers.views.isCalculationView(source)) { + throw new HTTPError("Cannot insert rows through a calculation view", 400) + } + if (!inputs._rev && !inputs._id) { inputs._id = db.generateRowID(inputs.tableId) } From 20f55e379581c497b91f9578954e965c8e3bc223 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 9 Oct 2024 12:29:17 +0100 Subject: [PATCH 05/31] Still fetch flags when the user is not logged in. --- .../backend-core/src/features/features.ts | 38 +++++++------ .../src/features/tests/features.spec.ts | 55 ++++++++++++++++++- 2 files changed, 75 insertions(+), 18 deletions(-) diff --git a/packages/backend-core/src/features/features.ts b/packages/backend-core/src/features/features.ts index 90a395d52a..305c53ff3a 100644 --- a/packages/backend-core/src/features/features.ts +++ b/packages/backend-core/src/features/features.ts @@ -1,7 +1,8 @@ import env from "../environment" +import * as crypto from "crypto" import * as context from "../context" import { PostHog, PostHogOptions } from "posthog-node" -import { FeatureFlag, IdentityType, UserCtx } from "@budibase/types" +import { FeatureFlag, UserCtx } from "@budibase/types" import tracer from "dd-trace" import { Duration } from "../utils" @@ -224,24 +225,29 @@ export class FlagSet, T extends { [key: string]: V }> { } const identity = context.getIdentity() - tags[`identity.type`] = identity?.type - tags[`identity.tenantId`] = identity?.tenantId - tags[`identity._id`] = identity?._id - if (posthog && identity?.type === IdentityType.USER) { + let userId = identity?._id + if (!userId && ctx?.ip) { + userId = crypto.createHash("sha512").update(ctx.ip).digest("hex") + } + + let tenantId = identity?.tenantId + if (!tenantId) { + tenantId = currentTenantId + } + + tags[`identity.type`] = identity?.type + tags[`identity._id`] = identity?._id + tags[`tenantId`] = tenantId + tags[`userId`] = userId + + if (posthog && userId) { tags[`readFromPostHog`] = true - const personProperties: Record = {} - if (identity.tenantId) { - personProperties.tenantId = identity.tenantId - } - - const posthogFlags = await posthog.getAllFlagsAndPayloads( - identity._id, - { - personProperties, - } - ) + const personProperties: Record = { tenantId } + const posthogFlags = await posthog.getAllFlagsAndPayloads(userId, { + personProperties, + }) for (const [name, value] of Object.entries(posthogFlags.featureFlags)) { if (!this.isFlagName(name)) { diff --git a/packages/backend-core/src/features/tests/features.spec.ts b/packages/backend-core/src/features/tests/features.spec.ts index 01c9bfa3c6..bf01ad67f3 100644 --- a/packages/backend-core/src/features/tests/features.spec.ts +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -4,6 +4,7 @@ import * as context from "../../context" import environment, { withEnv } from "../../environment" import nodeFetch from "node-fetch" import nock from "nock" +import * as crypto from "crypto" const schema = { TEST_BOOLEAN: Flag.boolean(false), @@ -27,10 +28,14 @@ interface PostHogFlags { featureFlagPayloads?: Record } -function mockPosthogFlags(flags: PostHogFlags) { +function mockPosthogFlags( + flags: PostHogFlags, + opts?: { token?: string; distinct_id?: string } +) { + const { token = "test", distinct_id = "us_1234" } = opts || {} nock("https://us.i.posthog.com") .post("/decide/?v=3", body => { - return body.token === "test" && body.distinct_id === "us_1234" + return body.token === token && body.distinct_id === distinct_id }) .reply(200, flags) .persist() @@ -214,6 +219,14 @@ describe("feature flags", () => { lastName: "User", } + // We need to pass in node-fetch here otherwise nock won't get used + // because posthog-node uses axios under the hood. + init({ + fetch: (url, opts) => { + return nodeFetch(url, opts) + }, + }) + nock("https://us.i.posthog.com") .post("/decide/?v=3", body => { return body.token === "test" && body.distinct_id === "us_1234" @@ -230,4 +243,42 @@ describe("feature flags", () => { } ) }) + + it("should still get flags when user is logged out", async () => { + const env: Partial = { + SELF_HOSTED: false, + POSTHOG_FEATURE_FLAGS_ENABLED: "true", + POSTHOG_API_HOST: "https://us.i.posthog.com", + POSTHOG_TOKEN: "test", + } + + const ctx = { ip: "127.0.0.1" } as UserCtx + const hashedIp = crypto.createHash("sha512").update(ctx.ip).digest("hex") + + await withEnv(env, async () => { + mockPosthogFlags( + { + featureFlags: { TEST_BOOLEAN: true }, + }, + { + distinct_id: hashedIp, + } + ) + + // We need to pass in node-fetch here otherwise nock won't get used + // because posthog-node uses axios under the hood. + init({ + fetch: (url, opts) => { + return nodeFetch(url, opts) + }, + }) + + await context.doInTenant("default", async () => { + const result = await flags.fetch(ctx) + expect(result.TEST_BOOLEAN).toBe(true) + }) + + shutdown() + }) + }) }) From eee2991b098bfc55d87aa032d02d372eb86b387d Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 9 Oct 2024 12:57:14 +0100 Subject: [PATCH 06/31] Rejib IP fetching. --- .../backend-core/src/context/mainContext.ts | 9 ++++ packages/backend-core/src/context/types.ts | 1 + .../backend-core/src/features/features.ts | 47 ++++--------------- packages/backend-core/src/middleware/index.ts | 1 + packages/backend-core/src/middleware/ip.ts | 12 +++++ packages/server/src/koa.ts | 1 + packages/worker/src/index.ts | 1 + 7 files changed, 33 insertions(+), 39 deletions(-) create mode 100644 packages/backend-core/src/middleware/ip.ts diff --git a/packages/backend-core/src/context/mainContext.ts b/packages/backend-core/src/context/mainContext.ts index 25b273e51c..64ba240fa5 100644 --- a/packages/backend-core/src/context/mainContext.ts +++ b/packages/backend-core/src/context/mainContext.ts @@ -253,6 +253,11 @@ export function getAppId(): string | undefined { } } +export function getIP(): string | undefined { + const context = Context.get() + return context?.ip +} + export const getProdAppId = () => { const appId = getAppId() if (!appId) { @@ -281,6 +286,10 @@ export function doInScimContext(task: any) { return newContext(updates, task) } +export function doInIPContext(ip: string, task: any) { + return newContext({ ip }, task) +} + export async function ensureSnippetContext(enabled = !env.isTest()) { const ctx = getCurrentContext() diff --git a/packages/backend-core/src/context/types.ts b/packages/backend-core/src/context/types.ts index ee84b49459..5549a47ff7 100644 --- a/packages/backend-core/src/context/types.ts +++ b/packages/backend-core/src/context/types.ts @@ -9,6 +9,7 @@ export type ContextMap = { identity?: IdentityContext environmentVariables?: Record isScim?: boolean + ip?: string automationId?: string isMigrating?: boolean vm?: VM diff --git a/packages/backend-core/src/features/features.ts b/packages/backend-core/src/features/features.ts index 305c53ff3a..5f2ba50f67 100644 --- a/packages/backend-core/src/features/features.ts +++ b/packages/backend-core/src/features/features.ts @@ -2,7 +2,7 @@ import env from "../environment" import * as crypto from "crypto" import * as context from "../context" import { PostHog, PostHogOptions } from "posthog-node" -import { FeatureFlag, UserCtx } from "@budibase/types" +import { FeatureFlag } from "@budibase/types" import tracer from "dd-trace" import { Duration } from "../utils" @@ -142,23 +142,17 @@ export class FlagSet, T extends { [key: string]: V }> { return this.flagSchema[name as keyof T] !== undefined } - async get( - key: K, - ctx?: UserCtx - ): Promise[K]> { - const flags = await this.fetch(ctx) + async get(key: K): Promise[K]> { + const flags = await this.fetch() return flags[key] } - async isEnabled>( - key: K, - ctx?: UserCtx - ): Promise { - const flags = await this.fetch(ctx) + async isEnabled>(key: K): Promise { + const flags = await this.fetch() return flags[key] } - async fetch(ctx?: UserCtx): Promise> { + async fetch(): Promise> { return await tracer.trace("features.fetch", async span => { const cachedFlags = context.getFeatureFlags>(this.setId) if (cachedFlags) { @@ -199,36 +193,11 @@ export class FlagSet, T extends { [key: string]: V }> { tags[`flags.${key}.source`] = "environment" } - const license = ctx?.user?.license - if (license) { - tags[`readFromLicense`] = true - - for (const feature of license.features) { - if (!this.isFlagName(feature)) { - continue - } - - if ( - flagValues[feature] === true || - specificallySetFalse.has(feature) - ) { - // If the flag is already set to through environment variables, we - // don't want to override it back to false here. - continue - } - - // @ts-expect-error - TS does not like you writing into a generic type, - // but we know that it's okay in this case because it's just an object. - flagValues[feature] = true - tags[`flags.${feature}.source`] = "license" - } - } - const identity = context.getIdentity() let userId = identity?._id - if (!userId && ctx?.ip) { - userId = crypto.createHash("sha512").update(ctx.ip).digest("hex") + if (!userId) { + userId = context.getIP() } let tenantId = identity?.tenantId diff --git a/packages/backend-core/src/middleware/index.ts b/packages/backend-core/src/middleware/index.ts index e1eb7f1d26..20c2125b13 100644 --- a/packages/backend-core/src/middleware/index.ts +++ b/packages/backend-core/src/middleware/index.ts @@ -20,3 +20,4 @@ export { default as correlation } from "../logging/correlation/middleware" export { default as errorHandling } from "./errorHandling" export { default as querystringToBody } from "./querystringToBody" export * as joiValidator from "./joi-validator" +export { default as ip } from "./ip" diff --git a/packages/backend-core/src/middleware/ip.ts b/packages/backend-core/src/middleware/ip.ts new file mode 100644 index 0000000000..69a0a2da38 --- /dev/null +++ b/packages/backend-core/src/middleware/ip.ts @@ -0,0 +1,12 @@ +import { Ctx } from "@budibase/types" +import { doInIPContext } from "../context" + +export default async (ctx: Ctx, next: any) => { + if (ctx.ip) { + doInIPContext(ctx.ip, () => { + return next() + }) + } else { + return next() + } +} diff --git a/packages/server/src/koa.ts b/packages/server/src/koa.ts index 9f90c04b50..dd8063795d 100644 --- a/packages/server/src/koa.ts +++ b/packages/server/src/koa.ts @@ -35,6 +35,7 @@ export default function createKoaApp() { app.use(middleware.correlation) app.use(middleware.pino) + app.use(middleware.ip) app.use(userAgent) const server = http.createServer(app.callback()) diff --git a/packages/worker/src/index.ts b/packages/worker/src/index.ts index 79e6f4493d..9c3c053baf 100644 --- a/packages/worker/src/index.ts +++ b/packages/worker/src/index.ts @@ -54,6 +54,7 @@ app.use(koaBody({ multipart: true })) app.use(koaSession(app)) app.use(middleware.correlation) app.use(middleware.pino) +app.use(middleware.ip) app.use(userAgent) // authentication From adad73a9e2210ed0986d70eab5b5a55f377216d5 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 9 Oct 2024 13:10:52 +0100 Subject: [PATCH 07/31] Fix tests. --- .../backend-core/src/features/features.ts | 5 ++- .../src/features/tests/features.spec.ts | 43 +++++-------------- packages/backend-core/src/middleware/ip.ts | 2 +- 3 files changed, 15 insertions(+), 35 deletions(-) diff --git a/packages/backend-core/src/features/features.ts b/packages/backend-core/src/features/features.ts index 5f2ba50f67..efe6495cb5 100644 --- a/packages/backend-core/src/features/features.ts +++ b/packages/backend-core/src/features/features.ts @@ -197,7 +197,10 @@ export class FlagSet, T extends { [key: string]: V }> { let userId = identity?._id if (!userId) { - userId = context.getIP() + const ip = context.getIP() + if (ip) { + userId = crypto.createHash("sha512").update(ip).digest("hex") + } } let tenantId = identity?.tenantId diff --git a/packages/backend-core/src/features/tests/features.spec.ts b/packages/backend-core/src/features/tests/features.spec.ts index bf01ad67f3..664490dcda 100644 --- a/packages/backend-core/src/features/tests/features.spec.ts +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -18,7 +18,6 @@ interface TestCase { identity?: Partial environmentFlags?: string posthogFlags?: PostHogFlags - licenseFlags?: Array expected?: Partial> errorMessage?: string | RegExp } @@ -117,17 +116,6 @@ describe("feature flags", () => { }, expected: { TEST_BOOLEAN: true }, }, - { - it: "should be able to set boolean flags through the license", - licenseFlags: ["TEST_BOOLEAN"], - expected: { TEST_BOOLEAN: true }, - }, - { - it: "should not be able to override a negative environment flag from license", - environmentFlags: "default:!TEST_BOOLEAN", - licenseFlags: ["TEST_BOOLEAN"], - expected: { TEST_BOOLEAN: false }, - }, { it: "should not error on unrecognised PostHog flag", posthogFlags: { @@ -135,18 +123,12 @@ describe("feature flags", () => { }, expected: flags.defaults(), }, - { - it: "should not error on unrecognised license flag", - licenseFlags: ["UNDEFINED"], - expected: flags.defaults(), - }, ])( "$it", async ({ identity, environmentFlags, posthogFlags, - licenseFlags, expected, errorMessage, }) => { @@ -162,8 +144,6 @@ describe("feature flags", () => { env.POSTHOG_API_HOST = "https://us.i.posthog.com" } - const ctx = { user: { license: { features: licenseFlags || [] } } } - await withEnv(env, async () => { // We need to pass in node-fetch here otherwise nock won't get used // because posthog-node uses axios under the hood. @@ -185,18 +165,13 @@ describe("feature flags", () => { await context.doInIdentityContext(fullIdentity, async () => { if (errorMessage) { - await expect(flags.fetch(ctx as UserCtx)).rejects.toThrow( - errorMessage - ) + await expect(flags.fetch()).rejects.toThrow(errorMessage) } else if (expected) { - const values = await flags.fetch(ctx as UserCtx) + const values = await flags.fetch() expect(values).toMatchObject(expected) for (const [key, expectedValue] of Object.entries(expected)) { - const value = await flags.get( - key as keyof typeof schema, - ctx as UserCtx - ) + const value = await flags.get(key as keyof typeof schema) expect(value).toBe(expectedValue) } } else { @@ -252,8 +227,8 @@ describe("feature flags", () => { POSTHOG_TOKEN: "test", } - const ctx = { ip: "127.0.0.1" } as UserCtx - const hashedIp = crypto.createHash("sha512").update(ctx.ip).digest("hex") + const ip = "127.0.0.1" + const hashedIp = crypto.createHash("sha512").update(ip).digest("hex") await withEnv(env, async () => { mockPosthogFlags( @@ -273,9 +248,11 @@ describe("feature flags", () => { }, }) - await context.doInTenant("default", async () => { - const result = await flags.fetch(ctx) - expect(result.TEST_BOOLEAN).toBe(true) + await context.doInIPContext(ip, async () => { + await context.doInTenant("default", async () => { + const result = await flags.fetch() + expect(result.TEST_BOOLEAN).toBe(true) + }) }) shutdown() diff --git a/packages/backend-core/src/middleware/ip.ts b/packages/backend-core/src/middleware/ip.ts index 69a0a2da38..940f644ad6 100644 --- a/packages/backend-core/src/middleware/ip.ts +++ b/packages/backend-core/src/middleware/ip.ts @@ -3,7 +3,7 @@ import { doInIPContext } from "../context" export default async (ctx: Ctx, next: any) => { if (ctx.ip) { - doInIPContext(ctx.ip, () => { + return await doInIPContext(ctx.ip, () => { return next() }) } else { From a4090243ecc0895a3bdb152d74187c0e67e70e43 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 9 Oct 2024 13:17:55 +0100 Subject: [PATCH 08/31] Fix lint. --- packages/backend-core/src/features/tests/features.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/features/tests/features.spec.ts b/packages/backend-core/src/features/tests/features.spec.ts index 664490dcda..9af8a8f4bb 100644 --- a/packages/backend-core/src/features/tests/features.spec.ts +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -1,4 +1,4 @@ -import { IdentityContext, IdentityType, UserCtx } from "@budibase/types" +import { IdentityContext, IdentityType } from "@budibase/types" import { Flag, FlagSet, FlagValues, init, shutdown } from "../" import * as context from "../../context" import environment, { withEnv } from "../../environment" From 26f2deb234596e3d273f4550023f5238774410dd Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 9 Oct 2024 13:34:43 +0100 Subject: [PATCH 09/31] Set proxy setting on Koa application. --- packages/backend-core/src/features/features.ts | 2 ++ packages/server/src/koa.ts | 1 + packages/worker/src/index.ts | 1 + 3 files changed, 4 insertions(+) diff --git a/packages/backend-core/src/features/features.ts b/packages/backend-core/src/features/features.ts index efe6495cb5..cb095e792c 100644 --- a/packages/backend-core/src/features/features.ts +++ b/packages/backend-core/src/features/features.ts @@ -198,6 +198,8 @@ export class FlagSet, T extends { [key: string]: V }> { let userId = identity?._id if (!userId) { const ip = context.getIP() + // TODO; REMOVE THIS + tags["userIP"] = ip if (ip) { userId = crypto.createHash("sha512").update(ip).digest("hex") } diff --git a/packages/server/src/koa.ts b/packages/server/src/koa.ts index dd8063795d..acae433cc3 100644 --- a/packages/server/src/koa.ts +++ b/packages/server/src/koa.ts @@ -12,6 +12,7 @@ import { userAgent } from "koa-useragent" export default function createKoaApp() { const app = new Koa() + app.proxy = true let mbNumber = parseInt(env.HTTP_MB_LIMIT || "10") if (!mbNumber || isNaN(mbNumber)) { diff --git a/packages/worker/src/index.ts b/packages/worker/src/index.ts index 9c3c053baf..fb6a97a844 100644 --- a/packages/worker/src/index.ts +++ b/packages/worker/src/index.ts @@ -46,6 +46,7 @@ bootstrap() const app: Application = new Application() app.keys = ["secret", "key"] +app.proxy = true // set up top level koa middleware app.use(handleScimBody) From 9d70b123e5bc2d8734a0f928b35944e2d3acf61f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 9 Oct 2024 15:22:38 +0100 Subject: [PATCH 10/31] Remove IP tag. --- packages/backend-core/src/features/features.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/backend-core/src/features/features.ts b/packages/backend-core/src/features/features.ts index cb095e792c..efe6495cb5 100644 --- a/packages/backend-core/src/features/features.ts +++ b/packages/backend-core/src/features/features.ts @@ -198,8 +198,6 @@ export class FlagSet, T extends { [key: string]: V }> { let userId = identity?._id if (!userId) { const ip = context.getIP() - // TODO; REMOVE THIS - tags["userIP"] = ip if (ip) { userId = crypto.createHash("sha512").update(ip).digest("hex") } From 3405e6d6b7ac795398b03d720267a0804c2d46f1 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 4 Oct 2024 16:12:01 +0100 Subject: [PATCH 11/31] Make new tables require ADMIN permissions to read and write. --- .../server/src/api/routes/tests/table.spec.ts | 9 ++++++- packages/server/src/utilities/security.ts | 26 ++++++++++++------- packages/types/src/documents/document.ts | 24 +++++++++++++++++ 3 files changed, 48 insertions(+), 11 deletions(-) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index d6c1651ef0..f82476d412 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -1,4 +1,4 @@ -import { context, docIds, events } from "@budibase/backend-core" +import { context, docIds, events, roles } from "@budibase/backend-core" import { PROTECTED_EXTERNAL_COLUMNS, PROTECTED_INTERNAL_COLUMNS, @@ -189,6 +189,13 @@ describe.each([ ) } ) + + it("should create tables with ADMIN read and write permissions", async () => { + const table = await config.api.table.save(tableForDatasource(datasource)) + const { permissions } = await config.api.permission.get(table._id!) + expect(permissions.read.role).toEqual(roles.BUILTIN_ROLE_IDS.ADMIN) + expect(permissions.write.role).toEqual(roles.BUILTIN_ROLE_IDS.ADMIN) + }) }) describe("update", () => { diff --git a/packages/server/src/utilities/security.ts b/packages/server/src/utilities/security.ts index 01a3468c9c..40dba465f1 100644 --- a/packages/server/src/utilities/security.ts +++ b/packages/server/src/utilities/security.ts @@ -1,5 +1,6 @@ import { permissions, roles } from "@budibase/backend-core" import { DocumentType, VirtualDocumentType } from "../db/utils" +import { getDocumentType, getVirtualDocumentType } from "@budibase/types" export const CURRENTLY_SUPPORTED_LEVELS: string[] = [ permissions.PermissionLevel.WRITE, @@ -8,14 +9,16 @@ export const CURRENTLY_SUPPORTED_LEVELS: string[] = [ ] export function getPermissionType(resourceId: string) { - const docType = Object.values(DocumentType).filter(docType => - resourceId.startsWith(docType) - )[0] - switch (docType as DocumentType | VirtualDocumentType) { - case DocumentType.TABLE: - case DocumentType.ROW: + const virtualDocType = getVirtualDocumentType(resourceId) + switch (virtualDocType) { case VirtualDocumentType.VIEW: return permissions.PermissionType.TABLE + } + + const docType = getDocumentType(resourceId) + switch (docType) { + case DocumentType.TABLE: + case DocumentType.ROW: case DocumentType.AUTOMATION: return permissions.PermissionType.AUTOMATION case DocumentType.WEBHOOK: @@ -39,15 +42,18 @@ export function getBasePermissions(resourceId: string) { if (!role.permissionId) { continue } + const perms = permissions.getBuiltinPermissionByID(role.permissionId) if (!perms) { continue } + const typedPermission = perms.permissions.find(perm => perm.type === type) - if ( - typedPermission && - CURRENTLY_SUPPORTED_LEVELS.indexOf(typedPermission.level) !== -1 - ) { + if (!typedPermission) { + continue + } + + if (CURRENTLY_SUPPORTED_LEVELS.includes(typedPermission.level)) { const level = typedPermission.level basePermissions[level] = roles.lowerBuiltinRoleID( basePermissions[level], diff --git a/packages/types/src/documents/document.ts b/packages/types/src/documents/document.ts index f5facfae9d..2f0da1081a 100644 --- a/packages/types/src/documents/document.ts +++ b/packages/types/src/documents/document.ts @@ -42,6 +42,17 @@ export enum DocumentType { ROW_ACTIONS = "ra", } +// Because DocumentTypes can overlap, we need to make sure that we search +// longest first to ensure we get the correct type. +const sortedDocumentTypes = Object.values(DocumentType).sort( + (a, b) => b.length - a.length // descending +) +export function getDocumentType(id: string): DocumentType | undefined { + return sortedDocumentTypes.find(docType => + id.startsWith(`${docType}${SEPARATOR}`) + ) +} + // these are the core documents that make up the data, design // and automation sections of an app. This excludes any internal // rows as we shouldn't import data. @@ -72,6 +83,19 @@ export enum VirtualDocumentType { ROW_ACTION = "row_action", } +// Because VirtualDocumentTypes can overlap, we need to make sure that we search +// longest first to ensure we get the correct type. +const sortedVirtualDocumentTypes = Object.values(VirtualDocumentType).sort( + (a, b) => b.length - a.length // descending +) +export function getVirtualDocumentType( + id: string +): VirtualDocumentType | undefined { + return sortedVirtualDocumentTypes.find(docType => + id.startsWith(`${docType}${SEPARATOR}`) + ) +} + export interface Document { _id?: string _rev?: string From 4cde2f26ad7a9c74ce041122d652cef5eebef9e0 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 8 Oct 2024 11:06:54 +0100 Subject: [PATCH 12/31] wip, tests broken --- .../server/src/api/controllers/permission.ts | 7 ++- .../src/api/routes/tests/permissions.spec.ts | 61 +++++++++++-------- .../server/src/api/routes/tests/table.spec.ts | 17 +++++- .../src/tests/utilities/api/permission.ts | 10 +++ packages/server/src/utilities/security.ts | 4 +- packages/types/src/api/web/app/permission.ts | 4 ++ 6 files changed, 73 insertions(+), 30 deletions(-) diff --git a/packages/server/src/api/controllers/permission.ts b/packages/server/src/api/controllers/permission.ts index c7afb6a351..668501b3f3 100644 --- a/packages/server/src/api/controllers/permission.ts +++ b/packages/server/src/api/controllers/permission.ts @@ -9,6 +9,7 @@ import { AddPermissionRequest, RemovePermissionRequest, RemovePermissionResponse, + FetchResourcePermissionInfoResponse, } from "@budibase/types" import { CURRENTLY_SUPPORTED_LEVELS, @@ -28,7 +29,9 @@ export function fetchLevels(ctx: UserCtx) { ctx.body = SUPPORTED_LEVELS } -export async function fetch(ctx: UserCtx) { +export async function fetch( + ctx: UserCtx +) { const db = context.getAppDB() const dbRoles: Role[] = await sdk.permissions.getAllDBRoles(db) let permissions: any = {} @@ -49,7 +52,7 @@ export async function fetch(ctx: UserCtx) { } } // apply the base permissions - const finalPermissions: Record> = {} + const finalPermissions: FetchResourcePermissionInfoResponse = {} for (let [resource, permission] of Object.entries(permissions)) { const basePerms = getBasePermissions(resource) finalPermissions[resource] = Object.assign(basePerms, permission) diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index b148d6fde1..f8974f44aa 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -12,7 +12,7 @@ const STD_ROLE_ID = BUILTIN_ROLE_IDS.PUBLIC describe("/permission", () => { let request = setup.getRequest() let config = setup.getConfig() - let table: Table & { _id: string } + let table: Table let perms: Document[] let row: Row let view: ViewV2 @@ -26,15 +26,33 @@ describe("/permission", () => { beforeEach(async () => { mocks.licenses.useCloudFree() - table = (await config.createTable()) as typeof table + table = await config.createTable() + + await config.api.permission.revoke({ + roleId: roles.BUILTIN_ROLE_IDS.ADMIN, + resourceId: table._id!, + level: PermissionLevel.READ, + }) + await config.api.permission.revoke({ + roleId: roles.BUILTIN_ROLE_IDS.ADMIN, + resourceId: table._id!, + level: PermissionLevel.WRITE, + }) + await config.api.permission.revoke({ + roleId: roles.BUILTIN_ROLE_IDS.ADMIN, + resourceId: table._id!, + level: PermissionLevel.EXECUTE, + }) + row = await config.createRow() view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), }) + perms = await config.api.permission.add({ roleId: STD_ROLE_ID, - resourceId: table._id, + resourceId: table._id!, level: PermissionLevel.READ, }) }) @@ -74,27 +92,22 @@ describe("/permission", () => { }) }) - it("should get resource permissions with multiple roles", async () => { + it.only("should get resource permissions with multiple roles", async () => { perms = await config.api.permission.add({ roleId: HIGHER_ROLE_ID, - resourceId: table._id, + resourceId: table._id!, level: PermissionLevel.WRITE, }) - const res = await config.api.permission.get(table._id) - expect(res).toEqual({ - permissions: { - read: { permissionType: "EXPLICIT", role: STD_ROLE_ID }, - write: { permissionType: "EXPLICIT", role: HIGHER_ROLE_ID }, - }, + const { permissions } = await config.api.permission.get(table._id!) + expect(permissions).toEqual({ + read: { permissionType: "EXPLICIT", role: STD_ROLE_ID }, + write: { permissionType: "EXPLICIT", role: HIGHER_ROLE_ID }, + execute: { permissionType: "BASE", role: "BASIC" }, }) - const allRes = await request - .get(`/api/permission`) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - expect(allRes.body[table._id]["read"]).toEqual(STD_ROLE_ID) - expect(allRes.body[table._id]["write"]).toEqual(HIGHER_ROLE_ID) + const all = await config.api.permission.fetch() + expect(all[table._id!]["read"]).toEqual(STD_ROLE_ID) + expect(all[table._id!]["write"]).toEqual(HIGHER_ROLE_ID) }) }) @@ -102,11 +115,11 @@ describe("/permission", () => { it("should be able to remove the permission", async () => { const res = await config.api.permission.revoke({ roleId: STD_ROLE_ID, - resourceId: table._id, + resourceId: table._id!, level: PermissionLevel.READ, }) expect(res[0]._id).toEqual(STD_ROLE_ID) - const permsRes = await config.api.permission.get(table._id) + const permsRes = await config.api.permission.get(table._id!) expect(permsRes.permissions[STD_ROLE_ID]).toBeUndefined() }) }) @@ -142,7 +155,7 @@ describe("/permission", () => { it("should not be able to access the view data when the table is not public and there are no view permissions overrides", async () => { await config.api.permission.revoke({ roleId: STD_ROLE_ID, - resourceId: table._id, + resourceId: table._id!, level: PermissionLevel.READ, }) @@ -167,7 +180,7 @@ describe("/permission", () => { }) await config.api.permission.revoke({ roleId: STD_ROLE_ID, - resourceId: table._id, + resourceId: table._id!, level: PermissionLevel.READ, }) // replicate changes before checking permissions @@ -179,8 +192,8 @@ describe("/permission", () => { it("shouldn't allow writing from a public user", async () => { const res = await request - .post(`/api/${table._id}/rows`) - .send(basicRow(table._id)) + .post(`/api/${table._id!}/rows`) + .send(basicRow(table._id!)) .set(config.publicHeaders()) .expect("Content-Type", /json/) .expect(401) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index f82476d412..60e8142522 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -21,6 +21,7 @@ import { ViewCalculation, ViewV2Enriched, RowExportFormat, + PermissionSource, } from "@budibase/types" import { checkBuilderEndpoint } from "./utilities/TestFunctions" import * as setup from "./utilities" @@ -193,8 +194,20 @@ describe.each([ it("should create tables with ADMIN read and write permissions", async () => { const table = await config.api.table.save(tableForDatasource(datasource)) const { permissions } = await config.api.permission.get(table._id!) - expect(permissions.read.role).toEqual(roles.BUILTIN_ROLE_IDS.ADMIN) - expect(permissions.write.role).toEqual(roles.BUILTIN_ROLE_IDS.ADMIN) + expect(permissions).toEqual({ + read: { + permissionType: PermissionSource.EXPLICIT, + role: roles.BUILTIN_ROLE_IDS.ADMIN, + }, + write: { + permissionType: PermissionSource.EXPLICIT, + role: roles.BUILTIN_ROLE_IDS.ADMIN, + }, + execute: { + permissionType: PermissionSource.EXPLICIT, + role: roles.BUILTIN_ROLE_IDS.ADMIN, + }, + }) }) }) diff --git a/packages/server/src/tests/utilities/api/permission.ts b/packages/server/src/tests/utilities/api/permission.ts index 986796d9a1..b4e641a1be 100644 --- a/packages/server/src/tests/utilities/api/permission.ts +++ b/packages/server/src/tests/utilities/api/permission.ts @@ -1,6 +1,7 @@ import { AddPermissionRequest, AddPermissionResponse, + FetchResourcePermissionInfoResponse, GetResourcePermsResponse, RemovePermissionRequest, RemovePermissionResponse, @@ -26,6 +27,15 @@ export class PermissionAPI extends TestAPI { ) } + fetch = async ( + expectations?: Expectations + ): Promise => { + return await this._get( + `/api/permission`, + { expectations } + ) + } + revoke = async ( request: RemovePermissionRequest, expectations?: Expectations diff --git a/packages/server/src/utilities/security.ts b/packages/server/src/utilities/security.ts index 40dba465f1..a3353f6184 100644 --- a/packages/server/src/utilities/security.ts +++ b/packages/server/src/utilities/security.ts @@ -35,9 +35,9 @@ export function getPermissionType(resourceId: string) { /** * works out the basic permissions based on builtin roles for a resource, using its ID */ -export function getBasePermissions(resourceId: string) { +export function getBasePermissions(resourceId: string): Record { const type = getPermissionType(resourceId) - const basePermissions: { [key: string]: string } = {} + const basePermissions: Record = {} for (let [roleId, role] of Object.entries(roles.getBuiltinRoles())) { if (!role.permissionId) { continue diff --git a/packages/types/src/api/web/app/permission.ts b/packages/types/src/api/web/app/permission.ts index bead2a4279..a5c4df5733 100644 --- a/packages/types/src/api/web/app/permission.ts +++ b/packages/types/src/api/web/app/permission.ts @@ -1,5 +1,9 @@ import { PermissionLevel } from "../../../sdk" +export interface FetchResourcePermissionInfoResponse { + [key: string]: Record +} + export interface ResourcePermissionInfo { role: string permissionType: string From 9f84262940f34178bc57a44e7b11231f0528e066 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 8 Oct 2024 13:12:37 +0200 Subject: [PATCH 13/31] Clean --- packages/server/src/api/controllers/permission.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/permission.ts b/packages/server/src/api/controllers/permission.ts index 668501b3f3..148cea401a 100644 --- a/packages/server/src/api/controllers/permission.ts +++ b/packages/server/src/api/controllers/permission.ts @@ -33,7 +33,7 @@ export async function fetch( ctx: UserCtx ) { const db = context.getAppDB() - const dbRoles: Role[] = await sdk.permissions.getAllDBRoles(db) + const dbRoles = await sdk.permissions.getAllDBRoles(db) let permissions: any = {} // create an object with structure role ID -> resource ID -> level for (let role of dbRoles) { From aa0a1737c8021508ecf6316001788a72c3d094b4 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 8 Oct 2024 13:16:17 +0200 Subject: [PATCH 14/31] Fix tests --- packages/server/src/api/controllers/permission.ts | 1 - packages/server/src/api/routes/tests/permissions.spec.ts | 3 +-- packages/server/src/utilities/security.ts | 1 + 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/controllers/permission.ts b/packages/server/src/api/controllers/permission.ts index 148cea401a..cbe260367c 100644 --- a/packages/server/src/api/controllers/permission.ts +++ b/packages/server/src/api/controllers/permission.ts @@ -1,7 +1,6 @@ import { permissions, roles, context } from "@budibase/backend-core" import { UserCtx, - Role, GetResourcePermsResponse, ResourcePermissionInfo, GetDependantResourcesResponse, diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index f8974f44aa..8601ed2df5 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -92,7 +92,7 @@ describe("/permission", () => { }) }) - it.only("should get resource permissions with multiple roles", async () => { + it("should get resource permissions with multiple roles", async () => { perms = await config.api.permission.add({ roleId: HIGHER_ROLE_ID, resourceId: table._id!, @@ -102,7 +102,6 @@ describe("/permission", () => { expect(permissions).toEqual({ read: { permissionType: "EXPLICIT", role: STD_ROLE_ID }, write: { permissionType: "EXPLICIT", role: HIGHER_ROLE_ID }, - execute: { permissionType: "BASE", role: "BASIC" }, }) const all = await config.api.permission.fetch() diff --git a/packages/server/src/utilities/security.ts b/packages/server/src/utilities/security.ts index a3353f6184..4f93c33ee4 100644 --- a/packages/server/src/utilities/security.ts +++ b/packages/server/src/utilities/security.ts @@ -19,6 +19,7 @@ export function getPermissionType(resourceId: string) { switch (docType) { case DocumentType.TABLE: case DocumentType.ROW: + return permissions.PermissionType.TABLE case DocumentType.AUTOMATION: return permissions.PermissionType.AUTOMATION case DocumentType.WEBHOOK: From 1e0902a8313273d522a1c2ffa8bc6e2cd5947f05 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 8 Oct 2024 13:40:15 +0200 Subject: [PATCH 15/31] Cleanup tess --- .../src/api/routes/tests/permissions.spec.ts | 277 +++++++++--------- 1 file changed, 137 insertions(+), 140 deletions(-) diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 8601ed2df5..b938bff279 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -12,10 +12,7 @@ const STD_ROLE_ID = BUILTIN_ROLE_IDS.PUBLIC describe("/permission", () => { let request = setup.getRequest() let config = setup.getConfig() - let table: Table let perms: Document[] - let row: Row - let view: ViewV2 afterAll(setup.afterAll) @@ -25,36 +22,6 @@ describe("/permission", () => { beforeEach(async () => { mocks.licenses.useCloudFree() - - table = await config.createTable() - - await config.api.permission.revoke({ - roleId: roles.BUILTIN_ROLE_IDS.ADMIN, - resourceId: table._id!, - level: PermissionLevel.READ, - }) - await config.api.permission.revoke({ - roleId: roles.BUILTIN_ROLE_IDS.ADMIN, - resourceId: table._id!, - level: PermissionLevel.WRITE, - }) - await config.api.permission.revoke({ - roleId: roles.BUILTIN_ROLE_IDS.ADMIN, - resourceId: table._id!, - level: PermissionLevel.EXECUTE, - }) - - row = await config.createRow() - view = await config.api.viewV2.create({ - tableId: table._id!, - name: generator.guid(), - }) - - perms = await config.api.permission.add({ - roleId: STD_ROLE_ID, - resourceId: table._id!, - level: PermissionLevel.READ, - }) }) describe("levels", () => { @@ -72,131 +39,161 @@ describe("/permission", () => { }) }) - describe("add", () => { - it("should be able to add permission to a role for the table", async () => { - expect(perms.length).toEqual(1) - expect(perms[0]._id).toEqual(`${STD_ROLE_ID}`) - }) + describe("table permissions", () => { + let table: Table & { _id: string } + let row: Row + let view: ViewV2 - it("should get the resource permissions", async () => { - const res = await request - .get(`/api/permission/${table._id}`) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - expect(res.body).toEqual({ - permissions: { - read: { permissionType: "EXPLICIT", role: STD_ROLE_ID }, - write: { permissionType: "BASE", role: HIGHER_ROLE_ID }, - }, + beforeEach(async () => { + mocks.licenses.useCloudFree() + + table = (await config.createTable()) as typeof table + row = await config.createRow() + view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), }) - }) - - it("should get resource permissions with multiple roles", async () => { perms = await config.api.permission.add({ - roleId: HIGHER_ROLE_ID, - resourceId: table._id!, - level: PermissionLevel.WRITE, - }) - const { permissions } = await config.api.permission.get(table._id!) - expect(permissions).toEqual({ - read: { permissionType: "EXPLICIT", role: STD_ROLE_ID }, - write: { permissionType: "EXPLICIT", role: HIGHER_ROLE_ID }, - }) - - const all = await config.api.permission.fetch() - expect(all[table._id!]["read"]).toEqual(STD_ROLE_ID) - expect(all[table._id!]["write"]).toEqual(HIGHER_ROLE_ID) - }) - }) - - describe("remove", () => { - it("should be able to remove the permission", async () => { - const res = await config.api.permission.revoke({ roleId: STD_ROLE_ID, - resourceId: table._id!, + resourceId: table._id, level: PermissionLevel.READ, }) - expect(res[0]._id).toEqual(STD_ROLE_ID) - const permsRes = await config.api.permission.get(table._id!) - expect(permsRes.permissions[STD_ROLE_ID]).toBeUndefined() - }) - }) - - describe("check public user allowed", () => { - it("should be able to read the row", async () => { - // replicate changes before checking permissions - await config.publish() - - const res = await request - .get(`/api/${table._id}/rows`) - .set(config.publicHeaders()) - .expect("Content-Type", /json/) - .expect(200) - expect(res.body[0]._id).toEqual(row._id) }) - it("should be able to access the view data when the table is set to public and with no view permissions overrides", async () => { - // Make view inherit table permissions. Needed for backwards compatibility with existing views. - await config.api.permission.revoke({ - roleId: STD_ROLE_ID, - resourceId: view.id, - level: PermissionLevel.READ, + describe("add", () => { + it("should be able to add permission to a role for the table", async () => { + expect(perms.length).toEqual(1) + expect(perms[0]._id).toEqual(`${STD_ROLE_ID}`) }) - // replicate changes before checking permissions - await config.publish() + it("should get the resource permissions", async () => { + const res = await request + .get(`/api/permission/${table._id}`) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(200) + expect(res.body).toEqual({ + permissions: { + read: { permissionType: "EXPLICIT", role: STD_ROLE_ID }, + write: { permissionType: "BASE", role: HIGHER_ROLE_ID }, + }, + }) + }) - const res = await config.api.viewV2.publicSearch(view.id) - expect(res.rows[0]._id).toEqual(row._id) + it("should get resource permissions with multiple roles", async () => { + perms = await config.api.permission.add({ + roleId: HIGHER_ROLE_ID, + resourceId: table._id, + level: PermissionLevel.WRITE, + }) + const res = await config.api.permission.get(table._id) + expect(res).toEqual({ + permissions: { + read: { permissionType: "EXPLICIT", role: STD_ROLE_ID }, + write: { permissionType: "EXPLICIT", role: HIGHER_ROLE_ID }, + }, + }) + + const allRes = await request + .get(`/api/permission`) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(200) + expect(allRes.body[table._id]["read"]).toEqual(STD_ROLE_ID) + expect(allRes.body[table._id]["write"]).toEqual(HIGHER_ROLE_ID) + }) }) - it("should not be able to access the view data when the table is not public and there are no view permissions overrides", async () => { - await config.api.permission.revoke({ - roleId: STD_ROLE_ID, - resourceId: table._id!, - level: PermissionLevel.READ, + describe("remove", () => { + it("should be able to remove the permission", async () => { + const res = await config.api.permission.revoke({ + roleId: STD_ROLE_ID, + resourceId: table._id, + level: PermissionLevel.READ, + }) + expect(res[0]._id).toEqual(STD_ROLE_ID) + const permsRes = await config.api.permission.get(table._id) + expect(permsRes.permissions[STD_ROLE_ID]).toBeUndefined() }) - - // Make view inherit table permissions. Needed for backwards compatibility with existing views. - await config.api.permission.revoke({ - roleId: STD_ROLE_ID, - resourceId: view.id, - level: PermissionLevel.READ, - }) - - // replicate changes before checking permissions - await config.publish() - - await config.api.viewV2.publicSearch(view.id, undefined, { status: 401 }) }) - it("should use the view permissions", async () => { - await config.api.permission.add({ - roleId: STD_ROLE_ID, - resourceId: view.id, - level: PermissionLevel.READ, - }) - await config.api.permission.revoke({ - roleId: STD_ROLE_ID, - resourceId: table._id!, - level: PermissionLevel.READ, - }) - // replicate changes before checking permissions - await config.publish() + describe("check public user allowed", () => { + it("should be able to read the row", async () => { + // replicate changes before checking permissions + await config.publish() - const res = await config.api.viewV2.publicSearch(view.id) - expect(res.rows[0]._id).toEqual(row._id) - }) + const res = await request + .get(`/api/${table._id}/rows`) + .set(config.publicHeaders()) + .expect("Content-Type", /json/) + .expect(200) + expect(res.body[0]._id).toEqual(row._id) + }) - it("shouldn't allow writing from a public user", async () => { - const res = await request - .post(`/api/${table._id!}/rows`) - .send(basicRow(table._id!)) - .set(config.publicHeaders()) - .expect("Content-Type", /json/) - .expect(401) - expect(res.status).toEqual(401) + it("should be able to access the view data when the table is set to public and with no view permissions overrides", async () => { + // Make view inherit table permissions. Needed for backwards compatibility with existing views. + await config.api.permission.revoke({ + roleId: STD_ROLE_ID, + resourceId: view.id, + level: PermissionLevel.READ, + }) + + // replicate changes before checking permissions + await config.publish() + + const res = await config.api.viewV2.publicSearch(view.id) + expect(res.rows[0]._id).toEqual(row._id) + }) + + it("should not be able to access the view data when the table is not public and there are no view permissions overrides", async () => { + await config.api.permission.revoke({ + roleId: STD_ROLE_ID, + resourceId: table._id, + level: PermissionLevel.READ, + }) + + // Make view inherit table permissions. Needed for backwards compatibility with existing views. + await config.api.permission.revoke({ + roleId: STD_ROLE_ID, + resourceId: view.id, + level: PermissionLevel.READ, + }) + + // replicate changes before checking permissions + await config.publish() + + await config.api.viewV2.publicSearch(view.id, undefined, { + status: 401, + }) + }) + + it("should use the view permissions", async () => { + await config.api.permission.add({ + roleId: STD_ROLE_ID, + resourceId: view.id, + level: PermissionLevel.READ, + }) + await config.api.permission.revoke({ + roleId: STD_ROLE_ID, + resourceId: table._id, + level: PermissionLevel.READ, + }) + // replicate changes before checking permissions + await config.publish() + + const res = await config.api.viewV2.publicSearch(view.id) + expect(res.rows[0]._id).toEqual(row._id) + }) + + it("shouldn't allow writing from a public user", async () => { + const res = await request + .post(`/api/${table._id}/rows`) + .send(basicRow(table._id)) + .set(config.publicHeaders()) + .expect("Content-Type", /json/) + .expect(401) + expect(res.status).toEqual(401) + }) }) }) From 38685b50df2a68fb19ac8c0b2e85a3e0530b571d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 8 Oct 2024 13:50:05 +0200 Subject: [PATCH 16/31] Add extra test --- .../src/api/routes/tests/permissions.spec.ts | 52 ++++++++++++------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index b938bff279..3b45d042c7 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -1,5 +1,5 @@ import { roles } from "@budibase/backend-core" -import { Document, PermissionLevel, Row, Table, ViewV2 } from "@budibase/types" +import { Document, PermissionLevel, Row, ViewV2 } from "@budibase/types" import * as setup from "./utilities" import { generator, mocks } from "@budibase/backend-core/tests" @@ -12,7 +12,6 @@ const STD_ROLE_ID = BUILTIN_ROLE_IDS.PUBLIC describe("/permission", () => { let request = setup.getRequest() let config = setup.getConfig() - let perms: Document[] afterAll(setup.afterAll) @@ -40,26 +39,43 @@ describe("/permission", () => { }) describe("table permissions", () => { - let table: Table & { _id: string } + let tableId: string let row: Row let view: ViewV2 + let perms: Document[] beforeEach(async () => { mocks.licenses.useCloudFree() - table = (await config.createTable()) as typeof table + const table = await config.createTable() + tableId = table._id! row = await config.createRow() view = await config.api.viewV2.create({ - tableId: table._id!, + tableId, name: generator.guid(), }) perms = await config.api.permission.add({ roleId: STD_ROLE_ID, - resourceId: table._id, + resourceId: tableId, level: PermissionLevel.READ, }) }) + it("tables should be defaulted to admin", async () => { + const table = await config.createTable() + const { permissions } = await config.api.permission.get(table._id!) + expect(permissions).toEqual({ + read: { + permissionType: "BASE", + role: "BASIC", + }, + write: { + permissionType: "BASE", + role: "BASIC", + }, + }) + }) + describe("add", () => { it("should be able to add permission to a role for the table", async () => { expect(perms.length).toEqual(1) @@ -68,7 +84,7 @@ describe("/permission", () => { it("should get the resource permissions", async () => { const res = await request - .get(`/api/permission/${table._id}`) + .get(`/api/permission/${tableId}`) .set(config.defaultHeaders()) .expect("Content-Type", /json/) .expect(200) @@ -83,10 +99,10 @@ describe("/permission", () => { it("should get resource permissions with multiple roles", async () => { perms = await config.api.permission.add({ roleId: HIGHER_ROLE_ID, - resourceId: table._id, + resourceId: tableId, level: PermissionLevel.WRITE, }) - const res = await config.api.permission.get(table._id) + const res = await config.api.permission.get(tableId) expect(res).toEqual({ permissions: { read: { permissionType: "EXPLICIT", role: STD_ROLE_ID }, @@ -99,8 +115,8 @@ describe("/permission", () => { .set(config.defaultHeaders()) .expect("Content-Type", /json/) .expect(200) - expect(allRes.body[table._id]["read"]).toEqual(STD_ROLE_ID) - expect(allRes.body[table._id]["write"]).toEqual(HIGHER_ROLE_ID) + expect(allRes.body[tableId]["read"]).toEqual(STD_ROLE_ID) + expect(allRes.body[tableId]["write"]).toEqual(HIGHER_ROLE_ID) }) }) @@ -108,11 +124,11 @@ describe("/permission", () => { it("should be able to remove the permission", async () => { const res = await config.api.permission.revoke({ roleId: STD_ROLE_ID, - resourceId: table._id, + resourceId: tableId, level: PermissionLevel.READ, }) expect(res[0]._id).toEqual(STD_ROLE_ID) - const permsRes = await config.api.permission.get(table._id) + const permsRes = await config.api.permission.get(tableId) expect(permsRes.permissions[STD_ROLE_ID]).toBeUndefined() }) }) @@ -123,7 +139,7 @@ describe("/permission", () => { await config.publish() const res = await request - .get(`/api/${table._id}/rows`) + .get(`/api/${tableId}/rows`) .set(config.publicHeaders()) .expect("Content-Type", /json/) .expect(200) @@ -148,7 +164,7 @@ describe("/permission", () => { it("should not be able to access the view data when the table is not public and there are no view permissions overrides", async () => { await config.api.permission.revoke({ roleId: STD_ROLE_ID, - resourceId: table._id, + resourceId: tableId, level: PermissionLevel.READ, }) @@ -175,7 +191,7 @@ describe("/permission", () => { }) await config.api.permission.revoke({ roleId: STD_ROLE_ID, - resourceId: table._id, + resourceId: tableId, level: PermissionLevel.READ, }) // replicate changes before checking permissions @@ -187,8 +203,8 @@ describe("/permission", () => { it("shouldn't allow writing from a public user", async () => { const res = await request - .post(`/api/${table._id}/rows`) - .send(basicRow(table._id)) + .post(`/api/${tableId}/rows`) + .send(basicRow(tableId)) .set(config.publicHeaders()) .expect("Content-Type", /json/) .expect(401) From 56459b27367780fbb739ab68b0be499e67f09b5d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 8 Oct 2024 15:47:35 +0200 Subject: [PATCH 17/31] Type --- packages/backend-core/src/security/permissions.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/backend-core/src/security/permissions.ts b/packages/backend-core/src/security/permissions.ts index 98704f16c6..4ed2cd3954 100644 --- a/packages/backend-core/src/security/permissions.ts +++ b/packages/backend-core/src/security/permissions.ts @@ -65,7 +65,13 @@ export enum BuiltinPermissionID { POWER = "power", } -export const BUILTIN_PERMISSIONS = { +export const BUILTIN_PERMISSIONS: { + [key in keyof typeof BuiltinPermissionID]: { + _id: (typeof BuiltinPermissionID)[key] + name: string + permissions: Permission[] + } +} = { PUBLIC: { _id: BuiltinPermissionID.PUBLIC, name: "Public", From 923f677bc60f8164d8d41ac5599aa76e5488b082 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Wed, 9 Oct 2024 14:57:15 +0000 Subject: [PATCH 18/31] Bump version to 2.32.15 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 9b765f17ae..4d36affd04 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.32.14", + "version": "2.32.15", "npmClient": "yarn", "packages": [ "packages/*", From ebd762cdb69a21d3edc5adee018f42bb5d1e730d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 8 Oct 2024 17:19:18 +0200 Subject: [PATCH 19/31] Fixes --- packages/backend-core/src/security/roles.ts | 4 ++-- packages/server/src/api/controllers/permission.ts | 4 ++-- packages/server/src/api/routes/tests/permissions.spec.ts | 8 +++++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 65339832cf..9ae3788aab 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -42,7 +42,7 @@ export class Role implements RoleDoc { _id: string _rev?: string name: string - permissionId: string + permissionId: BuiltinPermissionID inherits?: string version?: string permissions: Record = {} @@ -51,7 +51,7 @@ export class Role implements RoleDoc { constructor( id: string, name: string, - permissionId: string, + permissionId: BuiltinPermissionID, uiMetadata?: RoleUIMetadata ) { this._id = id diff --git a/packages/server/src/api/controllers/permission.ts b/packages/server/src/api/controllers/permission.ts index cbe260367c..a58d94ce80 100644 --- a/packages/server/src/api/controllers/permission.ts +++ b/packages/server/src/api/controllers/permission.ts @@ -33,7 +33,7 @@ export async function fetch( ) { const db = context.getAppDB() const dbRoles = await sdk.permissions.getAllDBRoles(db) - let permissions: any = {} + let permissions: Record> = {} // create an object with structure role ID -> resource ID -> level for (let role of dbRoles) { if (!role.permissions) { @@ -45,7 +45,7 @@ export async function fetch( } for (let [resource, levelArr] of Object.entries(role.permissions)) { const levels: string[] = Array.isArray(levelArr) ? levelArr : [levelArr] - const perms: Record = {} + const perms: Record = permissions[resource] || {} levels.forEach(level => (perms[level] = roleId!)) permissions[resource] = perms } diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 3b45d042c7..83452b4f1f 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -39,6 +39,8 @@ describe("/permission", () => { }) describe("table permissions", () => { + const DEFAULT_TABLE_ROLE_ID = BUILTIN_ROLE_IDS.ADMIN + let tableId: string let row: Row let view: ViewV2 @@ -67,11 +69,11 @@ describe("/permission", () => { expect(permissions).toEqual({ read: { permissionType: "BASE", - role: "BASIC", + role: DEFAULT_TABLE_ROLE_ID, }, write: { permissionType: "BASE", - role: "BASIC", + role: DEFAULT_TABLE_ROLE_ID, }, }) }) @@ -91,7 +93,7 @@ describe("/permission", () => { expect(res.body).toEqual({ permissions: { read: { permissionType: "EXPLICIT", role: STD_ROLE_ID }, - write: { permissionType: "BASE", role: HIGHER_ROLE_ID }, + write: { permissionType: "BASE", role: DEFAULT_TABLE_ROLE_ID }, }, }) }) From a0d3bf99931cad51c269d099b8b25c32d9b1ea2b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 8 Oct 2024 17:25:08 +0200 Subject: [PATCH 20/31] Remove redundant ok button --- .../backend/DataTable/modals/ManageAccessModal.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/builder/src/components/backend/DataTable/modals/ManageAccessModal.svelte b/packages/builder/src/components/backend/DataTable/modals/ManageAccessModal.svelte index 48b584690e..9f5cd9fd95 100644 --- a/packages/builder/src/components/backend/DataTable/modals/ManageAccessModal.svelte +++ b/packages/builder/src/components/backend/DataTable/modals/ManageAccessModal.svelte @@ -94,7 +94,7 @@ loadDependantInfo() - + Manage Access {#if requiresPlanToModify} From 37eb66e0d52875dc978cb5367d7773d2bc1b1dbc Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 8 Oct 2024 17:32:35 +0200 Subject: [PATCH 21/31] Cleanup tests --- .../src/api/routes/tests/permissions.spec.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 83452b4f1f..c8b7597c9d 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -43,19 +43,12 @@ describe("/permission", () => { let tableId: string let row: Row - let view: ViewV2 let perms: Document[] beforeEach(async () => { - mocks.licenses.useCloudFree() - const table = await config.createTable() tableId = table._id! row = await config.createRow() - view = await config.api.viewV2.create({ - tableId, - name: generator.guid(), - }) perms = await config.api.permission.add({ roleId: STD_ROLE_ID, resourceId: tableId, @@ -136,6 +129,15 @@ describe("/permission", () => { }) describe("check public user allowed", () => { + let view: ViewV2 + + beforeEach(async () => { + view = await config.api.viewV2.create({ + tableId, + name: generator.guid(), + }) + }) + it("should be able to read the row", async () => { // replicate changes before checking permissions await config.publish() From 577ab5b6ceaa68c6a58f37b7fcc5fb601b86679b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 8 Oct 2024 17:32:56 +0200 Subject: [PATCH 22/31] Fix tests --- packages/backend-core/src/security/permissions.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/backend-core/src/security/permissions.ts b/packages/backend-core/src/security/permissions.ts index 4ed2cd3954..aaf83f78df 100644 --- a/packages/backend-core/src/security/permissions.ts +++ b/packages/backend-core/src/security/permissions.ts @@ -84,7 +84,8 @@ export const BUILTIN_PERMISSIONS: { name: "Read only", permissions: [ new Permission(PermissionType.QUERY, PermissionLevel.READ), - new Permission(PermissionType.TABLE, PermissionLevel.READ), + // TODO: don't add "breaking changes" + // new Permission(PermissionType.TABLE, PermissionLevel.READ), new Permission(PermissionType.APP, PermissionLevel.READ), ], }, @@ -93,7 +94,7 @@ export const BUILTIN_PERMISSIONS: { name: "Read/Write", permissions: [ new Permission(PermissionType.QUERY, PermissionLevel.WRITE), - new Permission(PermissionType.TABLE, PermissionLevel.WRITE), + // new Permission(PermissionType.TABLE, PermissionLevel.WRITE), new Permission(PermissionType.AUTOMATION, PermissionLevel.EXECUTE), new Permission(PermissionType.LEGACY_VIEW, PermissionLevel.READ), new Permission(PermissionType.APP, PermissionLevel.READ), @@ -103,7 +104,7 @@ export const BUILTIN_PERMISSIONS: { _id: BuiltinPermissionID.POWER, name: "Power", permissions: [ - new Permission(PermissionType.TABLE, PermissionLevel.WRITE), + // new Permission(PermissionType.TABLE, PermissionLevel.WRITE), new Permission(PermissionType.USER, PermissionLevel.READ), new Permission(PermissionType.AUTOMATION, PermissionLevel.EXECUTE), new Permission(PermissionType.WEBHOOK, PermissionLevel.READ), From ff402c54e0ad79dde56eb568ee87573197bb56ff Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 8 Oct 2024 17:48:39 +0200 Subject: [PATCH 23/31] Add view tests --- .../src/api/routes/tests/permissions.spec.ts | 81 ++++++++++++++++--- packages/server/src/sdk/app/views/index.ts | 25 +----- 2 files changed, 69 insertions(+), 37 deletions(-) diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index c8b7597c9d..5dfece3126 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -1,5 +1,5 @@ import { roles } from "@budibase/backend-core" -import { Document, PermissionLevel, Row, ViewV2 } from "@budibase/types" +import { Document, PermissionLevel, Row } from "@budibase/types" import * as setup from "./utilities" import { generator, mocks } from "@budibase/backend-core/tests" @@ -9,6 +9,8 @@ const { BUILTIN_ROLE_IDS } = roles const HIGHER_ROLE_ID = BUILTIN_ROLE_IDS.BASIC const STD_ROLE_ID = BUILTIN_ROLE_IDS.PUBLIC +const DEFAULT_TABLE_ROLE_ID = BUILTIN_ROLE_IDS.ADMIN + describe("/permission", () => { let request = setup.getRequest() let config = setup.getConfig() @@ -39,16 +41,12 @@ describe("/permission", () => { }) describe("table permissions", () => { - const DEFAULT_TABLE_ROLE_ID = BUILTIN_ROLE_IDS.ADMIN - let tableId: string - let row: Row let perms: Document[] beforeEach(async () => { const table = await config.createTable() tableId = table._id! - row = await config.createRow() perms = await config.api.permission.add({ roleId: STD_ROLE_ID, resourceId: tableId, @@ -129,13 +127,16 @@ describe("/permission", () => { }) describe("check public user allowed", () => { - let view: ViewV2 + let viewId: string + let row: Row beforeEach(async () => { - view = await config.api.viewV2.create({ + const view = await config.api.viewV2.create({ tableId, name: generator.guid(), }) + viewId = view.id + row = await config.createRow() }) it("should be able to read the row", async () => { @@ -154,14 +155,14 @@ describe("/permission", () => { // Make view inherit table permissions. Needed for backwards compatibility with existing views. await config.api.permission.revoke({ roleId: STD_ROLE_ID, - resourceId: view.id, + resourceId: viewId, level: PermissionLevel.READ, }) // replicate changes before checking permissions await config.publish() - const res = await config.api.viewV2.publicSearch(view.id) + const res = await config.api.viewV2.publicSearch(viewId) expect(res.rows[0]._id).toEqual(row._id) }) @@ -175,14 +176,14 @@ describe("/permission", () => { // Make view inherit table permissions. Needed for backwards compatibility with existing views. await config.api.permission.revoke({ roleId: STD_ROLE_ID, - resourceId: view.id, + resourceId: viewId, level: PermissionLevel.READ, }) // replicate changes before checking permissions await config.publish() - await config.api.viewV2.publicSearch(view.id, undefined, { + await config.api.viewV2.publicSearch(viewId, undefined, { status: 401, }) }) @@ -190,7 +191,7 @@ describe("/permission", () => { it("should use the view permissions", async () => { await config.api.permission.add({ roleId: STD_ROLE_ID, - resourceId: view.id, + resourceId: viewId, level: PermissionLevel.READ, }) await config.api.permission.revoke({ @@ -201,7 +202,7 @@ describe("/permission", () => { // replicate changes before checking permissions await config.publish() - const res = await config.api.viewV2.publicSearch(view.id) + const res = await config.api.viewV2.publicSearch(viewId) expect(res.rows[0]._id).toEqual(row._id) }) @@ -217,6 +218,60 @@ describe("/permission", () => { }) }) + describe("view permissions", () => { + let tableId: string + let viewId: string + + beforeEach(async () => { + const table = await config.createTable() + tableId = table._id! + + const view = await config.api.viewV2.create({ + tableId, + name: generator.guid(), + }) + viewId = view.id + }) + + it("default permissions inherits the table default value", async () => { + const { permissions } = await config.api.permission.get(viewId) + expect(permissions).toEqual({ + read: { + permissionType: "INHERITED", + role: DEFAULT_TABLE_ROLE_ID, + inheritablePermission: DEFAULT_TABLE_ROLE_ID, + }, + write: { + permissionType: "INHERITED", + role: DEFAULT_TABLE_ROLE_ID, + inheritablePermission: DEFAULT_TABLE_ROLE_ID, + }, + }) + }) + + it("default permissions inherits explicit table permissions", async () => { + await config.api.permission.add({ + roleId: STD_ROLE_ID, + resourceId: tableId, + level: PermissionLevel.READ, + }) + + const { permissions } = await config.api.permission.get(viewId) + expect(permissions).toEqual({ + read: { + permissionType: "INHERITED", + role: STD_ROLE_ID, + inheritablePermission: STD_ROLE_ID, + }, + write: { + permissionType: "INHERITED", + role: DEFAULT_TABLE_ROLE_ID, + inheritablePermission: DEFAULT_TABLE_ROLE_ID, + }, + }) + }) + }) + describe("fetch builtins", () => { it("should be able to fetch builtin definitions", async () => { const res = await request diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 44f6beedb1..36d6dd6f85 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -3,7 +3,6 @@ import { canGroupBy, FieldType, isNumeric, - PermissionLevel, RelationSchemaField, RenameColumn, Table, @@ -13,7 +12,7 @@ import { ViewV2ColumnEnriched, ViewV2Enriched, } from "@budibase/types" -import { context, docIds, HTTPError, roles } from "@budibase/backend-core" +import { context, docIds, HTTPError } from "@budibase/backend-core" import { helpers, PROTECTED_EXTERNAL_COLUMNS, @@ -26,7 +25,6 @@ import { isExternalTableID } from "../../../integrations/utils" import * as internal from "./internal" import * as external from "./external" import sdk from "../../../sdk" -import { PermissionUpdateType, updatePermissionOnRole } from "../permissions" function pickApi(tableId: any) { if (isExternalTableID(tableId)) { @@ -245,27 +243,6 @@ export async function create( const view = await pickApi(tableId).create(tableId, viewRequest) - // Set permissions to be the same as the table - const tablePerms = await sdk.permissions.getResourcePerms(tableId) - const readRole = tablePerms[PermissionLevel.READ]?.role - const writeRole = tablePerms[PermissionLevel.WRITE]?.role - await updatePermissionOnRole( - { - roleId: readRole || roles.BUILTIN_ROLE_IDS.BASIC, - resourceId: view.id, - level: PermissionLevel.READ, - }, - PermissionUpdateType.ADD - ) - await updatePermissionOnRole( - { - roleId: writeRole || roles.BUILTIN_ROLE_IDS.BASIC, - resourceId: view.id, - level: PermissionLevel.WRITE, - }, - PermissionUpdateType.ADD - ) - return view } From 389c4a94673ec21a6bb37badeaeb81261e1413a9 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 8 Oct 2024 17:49:45 +0200 Subject: [PATCH 24/31] Add more tests --- .../src/api/routes/tests/permissions.spec.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 5dfece3126..44b57cf7cb 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -270,6 +270,28 @@ describe("/permission", () => { }, }) }) + + it("can sets permissions inherits explicit view permissions", async () => { + await config.api.permission.add({ + roleId: HIGHER_ROLE_ID, + resourceId: viewId, + level: PermissionLevel.WRITE, + }) + + const { permissions } = await config.api.permission.get(viewId) + expect(permissions).toEqual({ + read: { + permissionType: "INHERITED", + role: DEFAULT_TABLE_ROLE_ID, + inheritablePermission: DEFAULT_TABLE_ROLE_ID, + }, + write: { + permissionType: "EXPLICIT", + role: HIGHER_ROLE_ID, + inheritablePermission: DEFAULT_TABLE_ROLE_ID, + }, + }) + }) }) describe("fetch builtins", () => { From b3efea95bfc300347228377490f97f15481f2455 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 9 Oct 2024 12:31:47 +0200 Subject: [PATCH 25/31] Undo base permissions --- packages/backend-core/src/security/permissions.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/backend-core/src/security/permissions.ts b/packages/backend-core/src/security/permissions.ts index aaf83f78df..4ed2cd3954 100644 --- a/packages/backend-core/src/security/permissions.ts +++ b/packages/backend-core/src/security/permissions.ts @@ -84,8 +84,7 @@ export const BUILTIN_PERMISSIONS: { name: "Read only", permissions: [ new Permission(PermissionType.QUERY, PermissionLevel.READ), - // TODO: don't add "breaking changes" - // new Permission(PermissionType.TABLE, PermissionLevel.READ), + new Permission(PermissionType.TABLE, PermissionLevel.READ), new Permission(PermissionType.APP, PermissionLevel.READ), ], }, @@ -94,7 +93,7 @@ export const BUILTIN_PERMISSIONS: { name: "Read/Write", permissions: [ new Permission(PermissionType.QUERY, PermissionLevel.WRITE), - // new Permission(PermissionType.TABLE, PermissionLevel.WRITE), + new Permission(PermissionType.TABLE, PermissionLevel.WRITE), new Permission(PermissionType.AUTOMATION, PermissionLevel.EXECUTE), new Permission(PermissionType.LEGACY_VIEW, PermissionLevel.READ), new Permission(PermissionType.APP, PermissionLevel.READ), @@ -104,7 +103,7 @@ export const BUILTIN_PERMISSIONS: { _id: BuiltinPermissionID.POWER, name: "Power", permissions: [ - // new Permission(PermissionType.TABLE, PermissionLevel.WRITE), + new Permission(PermissionType.TABLE, PermissionLevel.WRITE), new Permission(PermissionType.USER, PermissionLevel.READ), new Permission(PermissionType.AUTOMATION, PermissionLevel.EXECUTE), new Permission(PermissionType.WEBHOOK, PermissionLevel.READ), From d01462221fb8e8da9e83bc3b5bf171b30a7bff44 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 9 Oct 2024 14:15:02 +0200 Subject: [PATCH 26/31] Set default permissions --- .../server/src/api/controllers/permission.ts | 9 ++++----- .../src/api/routes/tests/permissions.spec.ts | 20 +++++++------------ .../server/src/sdk/app/permissions/index.ts | 20 +++++++++++++++++++ packages/server/src/sdk/app/tables/create.ts | 8 ++++++++ packages/types/src/api/web/app/permission.ts | 4 ++-- 5 files changed, 41 insertions(+), 20 deletions(-) diff --git a/packages/server/src/api/controllers/permission.ts b/packages/server/src/api/controllers/permission.ts index a58d94ce80..ead48d3db8 100644 --- a/packages/server/src/api/controllers/permission.ts +++ b/packages/server/src/api/controllers/permission.ts @@ -94,18 +94,17 @@ export async function getDependantResources( export async function addPermission(ctx: UserCtx) { const params: AddPermissionRequest = ctx.params - ctx.body = await sdk.permissions.updatePermissionOnRole( - params, - PermissionUpdateType.ADD - ) + await sdk.permissions.updatePermissionOnRole(params, PermissionUpdateType.ADD) + ctx.status = 200 } export async function removePermission( ctx: UserCtx ) { const params: RemovePermissionRequest = ctx.params - ctx.body = await sdk.permissions.updatePermissionOnRole( + await sdk.permissions.updatePermissionOnRole( params, PermissionUpdateType.REMOVE ) + ctx.status = 200 } diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 44b57cf7cb..180f91fb42 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -42,12 +42,11 @@ describe("/permission", () => { describe("table permissions", () => { let tableId: string - let perms: Document[] beforeEach(async () => { const table = await config.createTable() tableId = table._id! - perms = await config.api.permission.add({ + await config.api.permission.add({ roleId: STD_ROLE_ID, resourceId: tableId, level: PermissionLevel.READ, @@ -59,11 +58,11 @@ describe("/permission", () => { const { permissions } = await config.api.permission.get(table._id!) expect(permissions).toEqual({ read: { - permissionType: "BASE", + permissionType: "EXPLICIT", role: DEFAULT_TABLE_ROLE_ID, }, write: { - permissionType: "BASE", + permissionType: "EXPLICIT", role: DEFAULT_TABLE_ROLE_ID, }, }) @@ -71,11 +70,6 @@ describe("/permission", () => { describe("add", () => { it("should be able to add permission to a role for the table", async () => { - expect(perms.length).toEqual(1) - expect(perms[0]._id).toEqual(`${STD_ROLE_ID}`) - }) - - it("should get the resource permissions", async () => { const res = await request .get(`/api/permission/${tableId}`) .set(config.defaultHeaders()) @@ -84,13 +78,13 @@ describe("/permission", () => { expect(res.body).toEqual({ permissions: { read: { permissionType: "EXPLICIT", role: STD_ROLE_ID }, - write: { permissionType: "BASE", role: DEFAULT_TABLE_ROLE_ID }, + write: { permissionType: "EXPLICIT", role: DEFAULT_TABLE_ROLE_ID }, }, }) }) it("should get resource permissions with multiple roles", async () => { - perms = await config.api.permission.add({ + await config.api.permission.add({ roleId: HIGHER_ROLE_ID, resourceId: tableId, level: PermissionLevel.WRITE, @@ -115,12 +109,12 @@ describe("/permission", () => { describe("remove", () => { it("should be able to remove the permission", async () => { - const res = await config.api.permission.revoke({ + await config.api.permission.revoke({ roleId: STD_ROLE_ID, resourceId: tableId, level: PermissionLevel.READ, }) - expect(res[0]._id).toEqual(STD_ROLE_ID) + const permsRes = await config.api.permission.get(tableId) expect(permsRes.permissions[STD_ROLE_ID]).toBeUndefined() }) diff --git a/packages/server/src/sdk/app/permissions/index.ts b/packages/server/src/sdk/app/permissions/index.ts index 2c3c0af95b..97af9ccc83 100644 --- a/packages/server/src/sdk/app/permissions/index.ts +++ b/packages/server/src/sdk/app/permissions/index.ts @@ -185,6 +185,26 @@ export async function updatePermissionOnRole( }) } +export async function setPermissions( + resourceId: string, + { + writeRole, + readRole, + }: { + writeRole: string + readRole: string + } +) { + await updatePermissionOnRole( + { roleId: writeRole, resourceId, level: PermissionLevel.WRITE }, + PermissionUpdateType.ADD + ) + await updatePermissionOnRole( + { roleId: readRole, resourceId, level: PermissionLevel.READ }, + PermissionUpdateType.ADD + ) +} + // utility function to stop this repetition - permissions always stored under roles export async function getAllDBRoles(db: Database) { const body = await db.allDocs( diff --git a/packages/server/src/sdk/app/tables/create.ts b/packages/server/src/sdk/app/tables/create.ts index ed6d6baeb0..0b15cdb15a 100644 --- a/packages/server/src/sdk/app/tables/create.ts +++ b/packages/server/src/sdk/app/tables/create.ts @@ -3,6 +3,8 @@ import { Row, Table } from "@budibase/types" import * as external from "./external" import * as internal from "./internal" import { isExternal } from "./utils" +import { setPermissions } from "../permissions" +import { roles } from "@budibase/backend-core" export async function create( table: Omit, @@ -15,5 +17,11 @@ export async function create( } else { createdTable = await internal.create(table, rows, userId) } + + await setPermissions(createdTable._id!, { + writeRole: roles.BUILTIN_ROLE_IDS.ADMIN, + readRole: roles.BUILTIN_ROLE_IDS.ADMIN, + }) + return createdTable } diff --git a/packages/types/src/api/web/app/permission.ts b/packages/types/src/api/web/app/permission.ts index a5c4df5733..b40310f21c 100644 --- a/packages/types/src/api/web/app/permission.ts +++ b/packages/types/src/api/web/app/permission.ts @@ -25,7 +25,7 @@ export interface AddedPermission { reason?: string } -export type AddPermissionResponse = AddedPermission[] +export interface AddPermissionResponse {} export interface AddPermissionRequest { roleId: string @@ -34,4 +34,4 @@ export interface AddPermissionRequest { } export interface RemovePermissionRequest extends AddPermissionRequest {} -export interface RemovePermissionResponse extends AddPermissionResponse {} +export interface RemovePermissionResponse {} From c84cda40b33c2c082b7dee171fc7c4df7bc3309a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 9 Oct 2024 14:36:37 +0200 Subject: [PATCH 27/31] Set default permissions to view --- .../src/api/routes/tests/permissions.spec.ts | 16 ++++++++-------- packages/server/src/sdk/app/views/index.ts | 8 ++++++++ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 180f91fb42..a479adb4cf 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -227,23 +227,23 @@ describe("/permission", () => { viewId = view.id }) - it("default permissions inherits the table default value", async () => { + it("default permissions inherits and persists the table default value", async () => { const { permissions } = await config.api.permission.get(viewId) expect(permissions).toEqual({ read: { - permissionType: "INHERITED", + permissionType: "EXPLICIT", role: DEFAULT_TABLE_ROLE_ID, inheritablePermission: DEFAULT_TABLE_ROLE_ID, }, write: { - permissionType: "INHERITED", + permissionType: "EXPLICIT", role: DEFAULT_TABLE_ROLE_ID, inheritablePermission: DEFAULT_TABLE_ROLE_ID, }, }) }) - it("default permissions inherits explicit table permissions", async () => { + it("does not update view permissions once persisted, even if table permissions change", async () => { await config.api.permission.add({ roleId: STD_ROLE_ID, resourceId: tableId, @@ -253,12 +253,12 @@ describe("/permission", () => { const { permissions } = await config.api.permission.get(viewId) expect(permissions).toEqual({ read: { - permissionType: "INHERITED", - role: STD_ROLE_ID, + permissionType: "EXPLICIT", + role: DEFAULT_TABLE_ROLE_ID, inheritablePermission: STD_ROLE_ID, }, write: { - permissionType: "INHERITED", + permissionType: "EXPLICIT", role: DEFAULT_TABLE_ROLE_ID, inheritablePermission: DEFAULT_TABLE_ROLE_ID, }, @@ -275,7 +275,7 @@ describe("/permission", () => { const { permissions } = await config.api.permission.get(viewId) expect(permissions).toEqual({ read: { - permissionType: "INHERITED", + permissionType: "EXPLICIT", role: DEFAULT_TABLE_ROLE_ID, inheritablePermission: DEFAULT_TABLE_ROLE_ID, }, diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 36d6dd6f85..629454fecc 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -3,6 +3,7 @@ import { canGroupBy, FieldType, isNumeric, + PermissionLevel, RelationSchemaField, RenameColumn, Table, @@ -243,6 +244,13 @@ export async function create( const view = await pickApi(tableId).create(tableId, viewRequest) + // Set permissions to be the same as the table + const tablePerms = await sdk.permissions.getResourcePerms(tableId) + await sdk.permissions.setPermissions(view.id, { + writeRole: tablePerms[PermissionLevel.WRITE].role, + readRole: tablePerms[PermissionLevel.READ].role, + }) + return view } From 85ef2f1d2c6e748cf4a7e3d689b292938ecdef7d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 9 Oct 2024 14:38:01 +0200 Subject: [PATCH 28/31] Fix build issue --- packages/backend-core/src/security/roles.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 9ae3788aab..65339832cf 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -42,7 +42,7 @@ export class Role implements RoleDoc { _id: string _rev?: string name: string - permissionId: BuiltinPermissionID + permissionId: string inherits?: string version?: string permissions: Record = {} @@ -51,7 +51,7 @@ export class Role implements RoleDoc { constructor( id: string, name: string, - permissionId: BuiltinPermissionID, + permissionId: string, uiMetadata?: RoleUIMetadata ) { this._id = id From 92372d1a4bbe0a164e91686a8bf1191f1724a449 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 9 Oct 2024 14:48:20 +0200 Subject: [PATCH 29/31] Fix test --- packages/server/src/api/routes/tests/table.spec.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 60e8142522..362ea24f8a 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -203,10 +203,6 @@ describe.each([ permissionType: PermissionSource.EXPLICIT, role: roles.BUILTIN_ROLE_IDS.ADMIN, }, - execute: { - permissionType: PermissionSource.EXPLICIT, - role: roles.BUILTIN_ROLE_IDS.ADMIN, - }, }) }) }) From a8476a1c8bc3b1fb8e27c6b9b3a53f6dc7ae3b73 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 9 Oct 2024 16:30:48 +0200 Subject: [PATCH 30/31] Fix tests --- packages/server/src/migrations/tests/index.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/migrations/tests/index.spec.ts b/packages/server/src/migrations/tests/index.spec.ts index d06cd37b69..6b3f3314ba 100644 --- a/packages/server/src/migrations/tests/index.spec.ts +++ b/packages/server/src/migrations/tests/index.spec.ts @@ -71,7 +71,7 @@ describe("migrations", () => { expect(events.datasource.created).toHaveBeenCalledTimes(2) expect(events.layout.created).toHaveBeenCalledTimes(1) expect(events.query.created).toHaveBeenCalledTimes(2) - expect(events.role.created).toHaveBeenCalledTimes(2) + expect(events.role.created).toHaveBeenCalledTimes(3) // created roles + admin (created on table creation) expect(events.table.created).toHaveBeenCalledTimes(3) expect(events.view.created).toHaveBeenCalledTimes(2) expect(events.view.calculationCreated).toHaveBeenCalledTimes(1) @@ -82,7 +82,7 @@ describe("migrations", () => { // to make sure caching is working as expected expect( events.processors.analyticsProcessor.processEvent - ).toHaveBeenCalledTimes(23) + ).toHaveBeenCalledTimes(24) // Addtion of of the events above }) }) }) From 6a0f80f28e71891a8ea4c3458b46a66db19839de Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 9 Oct 2024 17:13:46 +0200 Subject: [PATCH 31/31] Remove duplicated test --- .../server/src/api/routes/tests/table.spec.ts | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 362ea24f8a..d6c1651ef0 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -1,4 +1,4 @@ -import { context, docIds, events, roles } from "@budibase/backend-core" +import { context, docIds, events } from "@budibase/backend-core" import { PROTECTED_EXTERNAL_COLUMNS, PROTECTED_INTERNAL_COLUMNS, @@ -21,7 +21,6 @@ import { ViewCalculation, ViewV2Enriched, RowExportFormat, - PermissionSource, } from "@budibase/types" import { checkBuilderEndpoint } from "./utilities/TestFunctions" import * as setup from "./utilities" @@ -190,21 +189,6 @@ describe.each([ ) } ) - - it("should create tables with ADMIN read and write permissions", async () => { - const table = await config.api.table.save(tableForDatasource(datasource)) - const { permissions } = await config.api.permission.get(table._id!) - expect(permissions).toEqual({ - read: { - permissionType: PermissionSource.EXPLICIT, - role: roles.BUILTIN_ROLE_IDS.ADMIN, - }, - write: { - permissionType: PermissionSource.EXPLICIT, - role: roles.BUILTIN_ROLE_IDS.ADMIN, - }, - }) - }) }) describe("update", () => {