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/*", 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 90a395d52a..efe6495cb5 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 } from "@budibase/types" import tracer from "dd-trace" import { Duration } from "../utils" @@ -141,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) { @@ -198,50 +193,33 @@ export class FlagSet, T extends { [key: string]: V }> { tags[`flags.${key}.source`] = "environment" } - const license = ctx?.user?.license - if (license) { - tags[`readFromLicense`] = true + const identity = context.getIdentity() - 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" + let userId = identity?._id + if (!userId) { + const ip = context.getIP() + if (ip) { + userId = crypto.createHash("sha512").update(ip).digest("hex") } } - const identity = context.getIdentity() - tags[`identity.type`] = identity?.type - tags[`identity.tenantId`] = identity?.tenantId - tags[`identity._id`] = identity?._id + let tenantId = identity?.tenantId + if (!tenantId) { + tenantId = currentTenantId + } - if (posthog && identity?.type === IdentityType.USER) { + 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..9af8a8f4bb 100644 --- a/packages/backend-core/src/features/tests/features.spec.ts +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -1,9 +1,10 @@ -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" import nodeFetch from "node-fetch" import nock from "nock" +import * as crypto from "crypto" const schema = { TEST_BOOLEAN: Flag.boolean(false), @@ -17,7 +18,6 @@ interface TestCase { identity?: Partial environmentFlags?: string posthogFlags?: PostHogFlags - licenseFlags?: Array expected?: Partial> errorMessage?: string | RegExp } @@ -27,10 +27,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() @@ -112,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: { @@ -130,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, }) => { @@ -157,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. @@ -180,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 { @@ -214,6 +194,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 +218,44 @@ 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 ip = "127.0.0.1" + const hashedIp = crypto.createHash("sha512").update(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.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/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..940f644ad6 --- /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) { + return await doInIPContext(ctx.ip, () => { + return next() + }) + } else { + return next() + } +} 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", 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} diff --git a/packages/server/src/api/controllers/permission.ts b/packages/server/src/api/controllers/permission.ts index c7afb6a351..ead48d3db8 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, @@ -9,6 +8,7 @@ import { AddPermissionRequest, RemovePermissionRequest, RemovePermissionResponse, + FetchResourcePermissionInfoResponse, } from "@budibase/types" import { CURRENTLY_SUPPORTED_LEVELS, @@ -28,10 +28,12 @@ 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 = {} + const dbRoles = await sdk.permissions.getAllDBRoles(db) + let permissions: Record> = {} // create an object with structure role ID -> resource ID -> level for (let role of dbRoles) { if (!role.permissions) { @@ -43,13 +45,13 @@ export async function fetch(ctx: UserCtx) { } 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 } } // 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) @@ -92,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/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/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index b148d6fde1..a479adb4cf 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 } from "@budibase/types" import * as setup from "./utilities" import { generator, mocks } from "@budibase/backend-core/tests" @@ -9,13 +9,11 @@ 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() - let table: Table & { _id: string } - let perms: Document[] - let row: Row - let view: ViewV2 afterAll(setup.afterAll) @@ -25,18 +23,6 @@ describe("/permission", () => { 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(), - }) - perms = await config.api.permission.add({ - roleId: STD_ROLE_ID, - resourceId: table._id, - level: PermissionLevel.READ, - }) }) describe("levels", () => { @@ -54,137 +40,251 @@ 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 tableId: string - 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 }, - }, - }) - }) - - 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) - }) - }) - - 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() - }) - }) - - 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, - }) - - // 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 () => { + beforeEach(async () => { + const table = await config.createTable() + tableId = table._id! await config.api.permission.add({ roleId: STD_ROLE_ID, - resourceId: view.id, + resourceId: tableId, 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) + 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: "EXPLICIT", + role: DEFAULT_TABLE_ROLE_ID, + }, + write: { + permissionType: "EXPLICIT", + role: DEFAULT_TABLE_ROLE_ID, + }, + }) + }) + + describe("add", () => { + it("should be able to add permission to a role for the table", async () => { + const res = await request + .get(`/api/permission/${tableId}`) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(200) + expect(res.body).toEqual({ + permissions: { + read: { permissionType: "EXPLICIT", role: STD_ROLE_ID }, + write: { permissionType: "EXPLICIT", role: DEFAULT_TABLE_ROLE_ID }, + }, + }) + }) + + it("should get resource permissions with multiple roles", async () => { + await config.api.permission.add({ + roleId: HIGHER_ROLE_ID, + resourceId: tableId, + level: PermissionLevel.WRITE, + }) + const res = await config.api.permission.get(tableId) + 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[tableId]["read"]).toEqual(STD_ROLE_ID) + expect(allRes.body[tableId]["write"]).toEqual(HIGHER_ROLE_ID) + }) + }) + + describe("remove", () => { + it("should be able to remove the permission", async () => { + await config.api.permission.revoke({ + roleId: STD_ROLE_ID, + resourceId: tableId, + level: PermissionLevel.READ, + }) + + const permsRes = await config.api.permission.get(tableId) + expect(permsRes.permissions[STD_ROLE_ID]).toBeUndefined() + }) + }) + + describe("check public user allowed", () => { + let viewId: string + let row: Row + + beforeEach(async () => { + 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 () => { + // replicate changes before checking permissions + await config.publish() + + const res = await request + .get(`/api/${tableId}/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: viewId, + level: PermissionLevel.READ, + }) + + // replicate changes before checking permissions + await config.publish() + + const res = await config.api.viewV2.publicSearch(viewId) + 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: tableId, + 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: viewId, + level: PermissionLevel.READ, + }) + + // replicate changes before checking permissions + await config.publish() + + await config.api.viewV2.publicSearch(viewId, undefined, { + status: 401, + }) + }) + + it("should use the view permissions", async () => { + await config.api.permission.add({ + roleId: STD_ROLE_ID, + resourceId: viewId, + level: PermissionLevel.READ, + }) + await config.api.permission.revoke({ + roleId: STD_ROLE_ID, + resourceId: tableId, + level: PermissionLevel.READ, + }) + // replicate changes before checking permissions + await config.publish() + + const res = await config.api.viewV2.publicSearch(viewId) + expect(res.rows[0]._id).toEqual(row._id) + }) + + it("shouldn't allow writing from a public user", async () => { + const res = await request + .post(`/api/${tableId}/rows`) + .send(basicRow(tableId)) + .set(config.publicHeaders()) + .expect("Content-Type", /json/) + .expect(401) + expect(res.status).toEqual(401) + }) + }) + }) + + 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 and persists the table default value", async () => { + const { permissions } = await config.api.permission.get(viewId) + expect(permissions).toEqual({ + read: { + permissionType: "EXPLICIT", + role: DEFAULT_TABLE_ROLE_ID, + inheritablePermission: DEFAULT_TABLE_ROLE_ID, + }, + write: { + permissionType: "EXPLICIT", + role: DEFAULT_TABLE_ROLE_ID, + inheritablePermission: DEFAULT_TABLE_ROLE_ID, + }, + }) + }) + + 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, + level: PermissionLevel.READ, + }) + + const { permissions } = await config.api.permission.get(viewId) + expect(permissions).toEqual({ + read: { + permissionType: "EXPLICIT", + role: DEFAULT_TABLE_ROLE_ID, + inheritablePermission: STD_ROLE_ID, + }, + write: { + permissionType: "EXPLICIT", + role: DEFAULT_TABLE_ROLE_ID, + inheritablePermission: DEFAULT_TABLE_ROLE_ID, + }, + }) + }) + + 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: "EXPLICIT", + role: DEFAULT_TABLE_ROLE_ID, + inheritablePermission: DEFAULT_TABLE_ROLE_ID, + }, + write: { + permissionType: "EXPLICIT", + role: HIGHER_ROLE_ID, + inheritablePermission: DEFAULT_TABLE_ROLE_ID, + }, + }) }) }) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 75b409dced..195481ae10 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -26,6 +26,7 @@ import { NumericCalculationFieldMetadata, ViewV2Schema, ViewV2Type, + JsonTypes, } from "@budibase/types" import { generator, mocks } from "@budibase/backend-core/tests" import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" @@ -736,6 +737,69 @@ 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 && + 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: `Grouping by fields of type "${type}" is not supported`, + }, + } + ) + } + }) }) describe("update", () => { @@ -1914,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", () => { @@ -1978,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/koa.ts b/packages/server/src/koa.ts index 9f90c04b50..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)) { @@ -35,6 +36,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/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 }) }) }) 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/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) } 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/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 07f24d8f02..629454fecc 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -1,6 +1,8 @@ import { CalculationType, + canGroupBy, FieldType, + isNumeric, PermissionLevel, RelationSchemaField, RenameColumn, @@ -11,7 +13,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, @@ -24,7 +26,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)) { @@ -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 @@ -120,6 +121,13 @@ async function guardCalculationViewSchema( 400 ) } + + if (!canGroupBy(targetSchema.type)) { + throw new HTTPError( + `Grouping by fields of type "${targetSchema.type}" is not supported`, + 400 + ) + } } } @@ -238,24 +246,10 @@ export async function create( // 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 - ) + await sdk.permissions.setPermissions(view.id, { + writeRole: tablePerms[PermissionLevel.WRITE].role, + readRole: tablePerms[PermissionLevel.READ].role, + }) return view } 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 01a3468c9c..4f93c33ee4 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,13 +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) { + 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 VirtualDocumentType.VIEW: return permissions.PermissionType.TABLE case DocumentType.AUTOMATION: return permissions.PermissionType.AUTOMATION @@ -32,22 +36,25 @@ 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 } + 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/api/web/app/permission.ts b/packages/types/src/api/web/app/permission.ts index bead2a4279..b40310f21c 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 @@ -21,7 +25,7 @@ export interface AddedPermission { reason?: string } -export type AddPermissionResponse = AddedPermission[] +export interface AddPermissionResponse {} export interface AddPermissionRequest { roleId: string @@ -30,4 +34,4 @@ export interface AddPermissionRequest { } export interface RemovePermissionRequest extends AddPermissionRequest {} -export interface RemovePermissionResponse extends AddPermissionResponse {} +export interface RemovePermissionResponse {} 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 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 diff --git a/packages/worker/src/index.ts b/packages/worker/src/index.ts index 79e6f4493d..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) @@ -54,6 +55,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