From 64a5426d360c5ab29801c6f777d6c33bb57e9165 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 26 Jul 2023 17:32:21 +0100 Subject: [PATCH] Updates to remove app builder concept, denying access to app creation for app builders. --- .../backend-core/src/security/permissions.ts | 26 ++++--------------- packages/server/src/api/routes/application.ts | 2 +- packages/server/src/middleware/authorized.ts | 17 +++++++----- packages/server/src/middleware/builder.ts | 2 +- packages/types/src/documents/global/user.ts | 1 - packages/types/src/sdk/index.ts | 1 + packages/types/src/sdk/permissions.ts | 19 ++++++++++++++ .../src/api/controllers/global/users.ts | 23 ---------------- .../routes/global/tests/appBuilder.spec.ts | 22 ++-------------- .../worker/src/api/routes/global/users.ts | 3 +-- packages/worker/src/tests/api/users.ts | 15 ++--------- 11 files changed, 42 insertions(+), 89 deletions(-) create mode 100644 packages/types/src/sdk/permissions.ts diff --git a/packages/backend-core/src/security/permissions.ts b/packages/backend-core/src/security/permissions.ts index 6cacc12dd6..0edab188d9 100644 --- a/packages/backend-core/src/security/permissions.ts +++ b/packages/backend-core/src/security/permissions.ts @@ -1,29 +1,12 @@ -const { flatten } = require("lodash") -const { cloneDeep } = require("lodash/fp") +import { flatten } from "lodash" +import { cloneDeep } from "lodash/fp" +import { PermissionType, PermissionLevel } from "@budibase/types" +export { PermissionType, PermissionLevel } from "@budibase/types" export type RoleHierarchy = { permissionId: string }[] -export enum PermissionLevel { - READ = "read", - WRITE = "write", - EXECUTE = "execute", - ADMIN = "admin", -} - -// these are the global types, that govern the underlying default behaviour -export enum PermissionType { - APP = "app", - TABLE = "table", - USER = "user", - AUTOMATION = "automation", - WEBHOOK = "webhook", - BUILDER = "builder", - VIEW = "view", - QUERY = "query", -} - export class Permission { type: PermissionType level: PermissionLevel @@ -173,3 +156,4 @@ export function isPermissionLevelHigherThanRead(level: PermissionLevel) { // utility as a lot of things need simply the builder permission export const BUILDER = PermissionType.BUILDER +export const GLOBAL_BUILDER = PermissionType.GLOBAL_BUILDER diff --git a/packages/server/src/api/routes/application.ts b/packages/server/src/api/routes/application.ts index 0c1fa364ff..04d5f67b96 100644 --- a/packages/server/src/api/routes/application.ts +++ b/packages/server/src/api/routes/application.ts @@ -15,7 +15,7 @@ router ) .post( "/api/applications", - authorized(permissions.BUILDER), + authorized(permissions.GLOBAL_BUILDER), applicationValidator(), controller.create ) diff --git a/packages/server/src/middleware/authorized.ts b/packages/server/src/middleware/authorized.ts index dba5d47cb9..767c3d95ef 100644 --- a/packages/server/src/middleware/authorized.ts +++ b/packages/server/src/middleware/authorized.ts @@ -5,7 +5,7 @@ import { context, users, } from "@budibase/backend-core" -import { Role, UserCtx } from "@budibase/types" +import { Role, UserCtx, PermissionType, PermissionLevel } from "@budibase/types" import builderMiddleware from "./builder" import { isWebhookEndpoint } from "./utils" @@ -24,8 +24,8 @@ const csrf = auth.buildCsrfMiddleware() const checkAuthorized = async ( ctx: UserCtx, resourceRoles: any, - permType: any, - permLevel: any + permType: PermissionType, + permLevel: PermissionLevel ) => { const appId = context.getAppId() // check if this is a builder api and the user is not a builder @@ -47,10 +47,10 @@ const checkAuthorized = async ( } const checkAuthorizedResource = async ( - ctx: any, + ctx: UserCtx, resourceRoles: any, - permType: any, - permLevel: any + permType: PermissionType, + permLevel: PermissionLevel ) => { // get the user's roles const roleId = ctx.roleId || roles.BUILTIN_ROLE_IDS.PUBLIC @@ -122,7 +122,10 @@ export default ( // check general builder stuff, this middleware is a good way // to find API endpoints which are builder focused - if (permType === permissions.PermissionType.BUILDER) { + if ( + permType === permissions.PermissionType.BUILDER || + permType === permissions.PermissionType.GLOBAL_BUILDER + ) { await builderMiddleware(ctx) } diff --git a/packages/server/src/middleware/builder.ts b/packages/server/src/middleware/builder.ts index 881ec843a4..7df135c86a 100644 --- a/packages/server/src/middleware/builder.ts +++ b/packages/server/src/middleware/builder.ts @@ -10,7 +10,7 @@ import { setDebounce, } from "../utilities/redis" import { db as dbCore, cache } from "@budibase/backend-core" -import { UserCtx, Database, App } from "@budibase/types" +import { UserCtx, Database } from "@budibase/types" const DEBOUNCE_TIME_SEC = 30 diff --git a/packages/types/src/documents/global/user.ts b/packages/types/src/documents/global/user.ts index 3249660624..2ce714801d 100644 --- a/packages/types/src/documents/global/user.ts +++ b/packages/types/src/documents/global/user.ts @@ -43,7 +43,6 @@ export interface User extends Document { roles: UserRoles builder?: { global?: boolean - appBuilder?: boolean apps?: string[] } admin?: { diff --git a/packages/types/src/sdk/index.ts b/packages/types/src/sdk/index.ts index 49d0387a82..e4b5778ed9 100644 --- a/packages/types/src/sdk/index.ts +++ b/packages/types/src/sdk/index.ts @@ -18,3 +18,4 @@ export * from "./sso" export * from "./user" export * from "./cli" export * from "./websocket" +export * from "./permissions" diff --git a/packages/types/src/sdk/permissions.ts b/packages/types/src/sdk/permissions.ts new file mode 100644 index 0000000000..9e51e4c7b4 --- /dev/null +++ b/packages/types/src/sdk/permissions.ts @@ -0,0 +1,19 @@ +export enum PermissionLevel { + READ = "read", + WRITE = "write", + EXECUTE = "execute", + ADMIN = "admin", +} + +// these are the global types, that govern the underlying default behaviour +export enum PermissionType { + APP = "app", + TABLE = "table", + USER = "user", + AUTOMATION = "automation", + WEBHOOK = "webhook", + BUILDER = "builder", + GLOBAL_BUILDER = "globalBuilder", + VIEW = "view", + QUERY = "query", +} diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index d1e66b4ac1..99da08a5b8 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -433,26 +433,9 @@ export const inviteAccept = async ( } } -export const grantAppBuilder = async (ctx: Ctx) => { - const { userId } = ctx.params - const user = await userSdk.db.getUser(userId) - if (!user.builder) { - user.builder = {} - } - user.builder.appBuilder = true - await userSdk.db.save(user, { hashPassword: false }) - ctx.body = { message: `User "${user.email}" granted app builder permissions` } -} - export const addAppBuilder = async (ctx: Ctx) => { const { userId, appId } = ctx.params const user = await userSdk.db.getUser(userId) - if (!user.builder?.appBuilder && !userSdk.core.isGlobalBuilder(user)) { - ctx.throw( - 400, - "Unable to update access, user must be granted app builder permissions." - ) - } if (userSdk.core.isGlobalBuilder(user)) { ctx.body = { message: "User already admin - no permissions updated." } return @@ -472,12 +455,6 @@ export const addAppBuilder = async (ctx: Ctx) => { export const removeAppBuilder = async (ctx: Ctx) => { const { userId, appId } = ctx.params const user = await userSdk.db.getUser(userId) - if (!user.builder?.appBuilder && !userSdk.core.isGlobalBuilder(user)) { - ctx.throw( - 400, - "Unable to update access, user must be granted app builder permissions." - ) - } if (userSdk.core.isGlobalBuilder(user)) { ctx.body = { message: "User already admin - no permissions removed." } return diff --git a/packages/worker/src/api/routes/global/tests/appBuilder.spec.ts b/packages/worker/src/api/routes/global/tests/appBuilder.spec.ts index 83bc401759..22039471b4 100644 --- a/packages/worker/src/api/routes/global/tests/appBuilder.spec.ts +++ b/packages/worker/src/api/routes/global/tests/appBuilder.spec.ts @@ -24,27 +24,9 @@ describe("/api/global/users/:userId/app/builder", () => { return response.body as User } - async function grantAppBuilder(): Promise { - const user = await newUser() - await config.api.users.grantAppBuilder(user._id!) - return await getUser(user._id!) - } - - describe("POST /api/global/users/:userId/app/builder", () => { - it("should be able to grant a user builder permissions", async () => { - const user = await grantAppBuilder() - expect(user.builder?.appBuilder).toBe(true) - }) - }) - describe("PATCH /api/global/users/:userId/app/:appId/builder", () => { - it("shouldn't allow granting access to an app to a non-app builder", async () => { - const user = await newUser() - await config.api.users.grantBuilderToApp(user._id!, MOCK_APP_ID, 400) - }) - it("should be able to grant a user access to a particular app", async () => { - const user = await grantAppBuilder() + const user = await newUser() await config.api.users.grantBuilderToApp(user._id!, MOCK_APP_ID) const updated = await getUser(user._id!) expect(updated.builder?.appBuilder).toBe(true) @@ -54,7 +36,7 @@ describe("/api/global/users/:userId/app/builder", () => { describe("DELETE /api/global/users/:userId/app/:appId/builder", () => { it("should allow revoking access", async () => { - const user = await grantAppBuilder() + const user = await newUser() await config.api.users.grantBuilderToApp(user._id!, MOCK_APP_ID) let updated = await getUser(user._id!) expect(updated.builder?.apps![0]).toBe(MOCK_APP_ID) diff --git a/packages/worker/src/api/routes/global/users.ts b/packages/worker/src/api/routes/global/users.ts index 9c1b5d9acb..348939a2e7 100644 --- a/packages/worker/src/api/routes/global/users.ts +++ b/packages/worker/src/api/routes/global/users.ts @@ -122,8 +122,7 @@ router buildAdminInitValidation(), controller.adminUser ) - .post("/api/global/users/:userId/app/builder", controller.grantAppBuilder) - .patch( + .post( "/api/global/users/:userId/app/:appId/builder", controller.addAppBuilder ) diff --git a/packages/worker/src/tests/api/users.ts b/packages/worker/src/tests/api/users.ts index bafc157e69..d9f6595f1d 100644 --- a/packages/worker/src/tests/api/users.ts +++ b/packages/worker/src/tests/api/users.ts @@ -141,30 +141,19 @@ export class UserAPI extends TestAPI { .expect(opts?.status ? opts.status : 200) } - grantAppBuilder = (userId: string) => { - return this.request - .post(`/api/global/users/${userId}/app/builder`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - } - grantBuilderToApp = ( userId: string, appId: string, statusCode: number = 200 ) => { return this.request - .patch(`/api/global/users/${userId}/app/${appId}/builder`) + .post(`/api/global/users/${userId}/app/${appId}/builder`) .set(this.config.defaultHeaders()) .expect("Content-Type", /json/) .expect(statusCode) } - revokeBuilderToApp = ( - userId: string, - appId: string - ) => { + revokeBuilderToApp = (userId: string, appId: string) => { return this.request .delete(`/api/global/users/${userId}/app/${appId}/builder`) .set(this.config.defaultHeaders())