Merge pull request #11581 from Budibase/feature/group-per-app-builders

Group support for per app builders

jestTestcontainersConfigGenerator.js

@@ -9,18 +9,8 @@ module.exports = () => {
       },
       wait: {
         type: "ports",
-        timeout: 10000,
+        timeout: 20000,
       }
     }
   }
 }
-
-// module.exports = () => {
-//   return {
-//     dockerCompose: {
-//       composeFilePath: "../../hosting",
-//       composeFile: "docker-compose.test.yaml",
-//       startupTimeout: 10000,
-//     },
-//   }
-// }

packages/backend-core/src/cache/user.ts

@@ -4,6 +4,8 @@ import * as context from "../context"
 import * as platform from "../platform"
 import env from "../environment"
 import * as accounts from "../accounts"
+import { UserDB } from "../users"
+import { sdk } from "@budibase/shared-core"
 
 const EXPIRY_SECONDS = 3600
 
@@ -60,6 +62,18 @@ export async function getUser(
     // make sure the tenant ID is always correct/set
     user.tenantId = tenantId
   }
+  // if has groups, could have builder permissions granted by a group
+  if (user.userGroups && !sdk.users.isGlobalBuilder(user)) {
+    await context.doInTenant(tenantId, async () => {
+      const appIds = await UserDB.getGroupBuilderAppIds(user)
+      if (appIds.length) {
+        const existing = user.builder?.apps || []
+        user.builder = {
+          apps: [...new Set(existing.concat(appIds))],
+        }
+      }
+    })
+  }
   return user
 }
 

packages/backend-core/src/middleware/builderOnly.ts

@@ -5,7 +5,8 @@ import env from "../environment"
 
 export default async (ctx: UserCtx, next: any) => {
   const appId = getAppId()
-  const builderFn = env.isWorker()
+  const builderFn =
+    env.isWorker() || !appId
       ? hasBuilderPermissions
       : env.isApps()
       ? isBuilder

packages/backend-core/src/middleware/builderOrAdmin.ts

@@ -5,7 +5,8 @@ import env from "../environment"
 
 export default async (ctx: UserCtx, next: any) => {
   const appId = getAppId()
-  const builderFn = env.isWorker()
+  const builderFn =
+    env.isWorker() || !appId
       ? hasBuilderPermissions
       : env.isApps()
       ? isBuilder

packages/backend-core/src/users/db.ts

@@ -20,6 +20,8 @@ import {
   SaveUserOpts,
   User,
   UserStatus,
+  UserGroup,
+  ContextUser,
 } from "@budibase/types"
 import {
   getAccountHolderFromUserIds,
@@ -32,8 +34,14 @@ import { hash } from "../utils"
 type QuotaUpdateFn = (change: number, cb?: () => Promise<any>) => Promise<any>
 type GroupUpdateFn = (groupId: string, userIds: string[]) => Promise<any>
 type FeatureFn = () => Promise<Boolean>
+type GroupGetFn = (ids: string[]) => Promise<UserGroup[]>
+type GroupBuildersFn = (user: User) => Promise<string[]>
 type QuotaFns = { addUsers: QuotaUpdateFn; removeUsers: QuotaUpdateFn }
-type GroupFns = { addUsers: GroupUpdateFn }
+type GroupFns = {
+  addUsers: GroupUpdateFn
+  getBulk: GroupGetFn
+  getGroupBuilderAppIds: GroupBuildersFn
+}
 type FeatureFns = { isSSOEnforced: FeatureFn; isAppBuildersEnabled: FeatureFn }
 
 const bulkDeleteProcessing = async (dbUser: User) => {
@@ -465,4 +473,12 @@ export class UserDB {
     await cache.user.invalidateUser(userId)
     await sessions.invalidateSessions(userId, { reason: "deletion" })
   }
+
+  static async getGroups(groupIds: string[]) {
+    return await this.groups.getBulk(groupIds)
+  }
+
+  static async getGroupBuilderAppIds(user: User) {
+    return await this.groups.getGroupBuilderAppIds(user)
+  }
 }

packages/pro

@@ -1 +1 @@
-Subproject commit 06a28b18a409cc12e9e8a5b69a094adcc6babd5a
+Subproject commit af8a40089809485712c7ef626d9e04090ef09975

packages/server/src/api/controllers/auth.ts

@@ -2,9 +2,9 @@ import { outputProcessing } from "../../utilities/rowProcessor"
 import { InternalTables } from "../../db/utils"
 import { getFullUser } from "../../utilities/users"
 import { roles, context } from "@budibase/backend-core"
-import { groups } from "@budibase/pro"
-import { ContextUser, User, Row, UserCtx } from "@budibase/types"
+import { ContextUser, Row, UserCtx } from "@budibase/types"
 import sdk from "../../sdk"
+import { processUser } from "../../utilities/global"
 
 const PUBLIC_ROLE = roles.BUILTIN_ROLE_IDS.PUBLIC
 
@@ -26,7 +26,7 @@ export async function fetchSelf(ctx: UserCtx) {
   }
 
   const appId = context.getAppId()
-  const user: ContextUser = await getFullUser(ctx, userId)
+  let user: ContextUser = await getFullUser(ctx, userId)
   // this shouldn't be returned by the app self
   delete user.roles
   // forward the csrf token from the session
@@ -36,8 +36,7 @@ export async function fetchSelf(ctx: UserCtx) {
     const db = context.getAppDB()
     // check for group permissions
     if (!user.roleId || user.roleId === PUBLIC_ROLE) {
-      const groupRoleId = await groups.getGroupRoleId(user as User, appId)
-      user.roleId = groupRoleId || user.roleId
+      user = await processUser(user, { appId })
     }
     // remove the full roles structure
     delete user.roles

packages/server/src/integration-test/postgres.spec.ts

@@ -18,7 +18,7 @@ import {
 import _ from "lodash"
 import { generator } from "@budibase/backend-core/tests"
 import { utils } from "@budibase/backend-core"
-import { GenericContainer } from "testcontainers"
+import { GenericContainer, Wait, StartedTestContainer } from "testcontainers"
 
 const config = setup.getConfig()!
 
@@ -37,22 +37,36 @@ describe("postgres integrations", () => {
 
   let host: string
   let port: number
+  const containers: StartedTestContainer[] = []
 
   beforeAll(async () => {
-    const container = await new GenericContainer("postgres")
+    const containerPostgres = await new GenericContainer("postgres")
       .withExposedPorts(5432)
       .withEnv("POSTGRES_PASSWORD", "password")
+      .withWaitStrategy(
+        Wait.forLogMessage(
+          "PostgreSQL init process complete; ready for start up."
+        )
+      )
       .start()
 
-    host = container.getContainerIpAddress()
-    port = container.getMappedPort(5432)
+    host = containerPostgres.getContainerIpAddress()
+    port = containerPostgres.getMappedPort(5432)
 
     await config.init()
     const apiKey = await config.generateApiKey()
 
+    containers.push(containerPostgres)
+
     makeRequest = generateMakeRequest(apiKey, true)
   })
 
+  afterAll(async () => {
+    for (let container of containers) {
+      await container.stop()
+    }
+  })
+
   function pgDatasourceConfig() {
     return {
       datasource: {

packages/server/src/utilities/global.ts

@@ -12,75 +12,65 @@ import { groups } from "@budibase/pro"
 import { UserCtx, ContextUser, User, UserGroup } from "@budibase/types"
 import cloneDeep from "lodash/cloneDeep"
 
-export function updateAppRole(
+export async function processUser(
   user: ContextUser,
-  { appId }: { appId?: string } = {}
+  opts: { appId?: string; groups?: UserGroup[] } = {}
 ) {
-  appId = appId || context.getAppId()
-
   if (!user || (!user.roles && !user.userGroups)) {
     return user
   }
-  // if in an multi-tenancy environment make sure roles are never updated
+  user = cloneDeep(user)
+  delete user.password
+  const appId = opts.appId || context.getAppId()
+  if (!appId) {
+    throw new Error("Unable to process user without app ID")
+  }
+  // if in a multi-tenancy environment and in wrong tenant make sure roles are never updated
   if (env.MULTI_TENANCY && appId && !tenancy.isUserInAppTenant(appId, user)) {
     user = users.removePortalUserPermissions(user)
     user.roleId = roles.BUILTIN_ROLE_IDS.PUBLIC
     return user
   }
-  // always use the deployed app
-  if (appId && user.roles) {
+  let groupList: UserGroup[] = []
+  if (appId && user?.userGroups?.length) {
+    groupList = opts.groups
+      ? opts.groups
+      : await groups.getBulk(user.userGroups)
+  }
+  // check if a group provides builder access
+  const builderAppIds = await groups.getGroupBuilderAppIds(user, {
+    appId,
+    groups: groupList,
+  })
+  if (builderAppIds.length && !users.isBuilder(user, appId)) {
+    const existingApps = user.builder?.apps || []
+    user.builder = {
+      apps: [...new Set(existingApps.concat(builderAppIds))],
+    }
+  }
+  // builders are always admins within the app
+  if (users.isBuilder(user, appId)) {
+    user.roleId = roles.BUILTIN_ROLE_IDS.ADMIN
+  }
+  // try to get the role from the user list
+  if (!user.roleId && appId && user.roles) {
     user.roleId = user.roles[dbCore.getProdAppID(appId)]
   }
-  // if a role wasn't found then either set as admin (builder) or public (everyone else)
-  if (!user.roleId && users.isBuilder(user, appId)) {
-    user.roleId = roles.BUILTIN_ROLE_IDS.ADMIN
-  } else if (!user.roleId && !user?.userGroups?.length) {
-    user.roleId = roles.BUILTIN_ROLE_IDS.PUBLIC
-  }
-
-  delete user.roles
-  return user
-}
-
-async function checkGroupRoles(
-  user: ContextUser,
-  opts: { appId?: string; groups?: UserGroup[] } = {}
-) {
-  if (user.roleId && user.roleId !== roles.BUILTIN_ROLE_IDS.PUBLIC) {
-    return user
-  }
-  if (opts.appId) {
-    user.roleId = await groups.getGroupRoleId(user as User, opts.appId, {
-      groups: opts.groups,
+  // try to get the role from the group list
+  if (!user.roleId && groupList) {
+    user.roleId = await groups.getGroupRoleId(user, appId, {
+      groups: groupList,
     })
   }
   // final fallback, simply couldn't find a role - user must be public
   if (!user.roleId) {
     user.roleId = roles.BUILTIN_ROLE_IDS.PUBLIC
   }
+  // remove the roles as it is now set
+  delete user.roles
   return user
 }
 
-export async function processUser(
-  user: ContextUser,
-  opts: { appId?: string; groups?: UserGroup[] } = {}
-) {
-  let clonedUser = cloneDeep(user)
-  if (clonedUser) {
-    delete clonedUser.password
-  }
-  const appId = opts.appId || context.getAppId()
-  clonedUser = updateAppRole(clonedUser, { appId })
-  if (!clonedUser.roleId && clonedUser?.userGroups?.length) {
-    clonedUser = await checkGroupRoles(clonedUser, {
-      appId,
-      groups: opts?.groups,
-    })
-  }
-
-  return clonedUser
-}
-
 export async function getCachedSelf(ctx: UserCtx, appId: string) {
   // this has to be tenant aware, can't depend on the context to find it out
   // running some middlewares before the tenancy causes context to break

packages/server/src/utilities/workerRequests.ts

@@ -8,10 +8,9 @@ import {
   logging,
   env as coreEnv,
 } from "@budibase/backend-core"
-import { updateAppRole } from "./global"
-import { BBContext, User, EmailInvite } from "@budibase/types"
+import { Ctx, User, EmailInvite } from "@budibase/types"
 
-export function request(ctx?: BBContext, request?: any) {
+export function request(ctx?: Ctx, request?: any) {
   if (!request.headers) {
     request.headers = {}
   }
@@ -43,7 +42,7 @@ export function request(ctx?: BBContext, request?: any) {
 async function checkResponse(
   response: any,
   errorMsg: string,
-  { ctx }: { ctx?: BBContext } = {}
+  { ctx }: { ctx?: Ctx } = {}
 ) {
   if (response.status !== 200) {
     let error
@@ -105,21 +104,7 @@ export async function sendSmtpEmail({
   return checkResponse(response, "send email")
 }
 
-export async function getGlobalSelf(ctx: BBContext, appId?: string) {
-  const endpoint = `/api/global/self`
-  const response = await fetch(
-    checkSlashesInUrl(env.WORKER_URL + endpoint),
-    // we don't want to use API key when getting self
-    request(ctx, { method: "GET" })
-  )
-  let json = await checkResponse(response, "get self globally", { ctx })
-  if (appId) {
-    json = updateAppRole(json)
-  }
-  return json
-}
-
-export async function removeAppFromUserRoles(ctx: BBContext, appId: string) {
+export async function removeAppFromUserRoles(ctx: Ctx, appId: string) {
   const prodAppId = dbCore.getProdAppID(appId)
   const response = await fetch(
     checkSlashesInUrl(env.WORKER_URL + `/api/global/roles/${prodAppId}`),
@@ -130,7 +115,7 @@ export async function removeAppFromUserRoles(ctx: BBContext, appId: string) {
   return checkResponse(response, "remove app role")
 }
 
-export async function allGlobalUsers(ctx: BBContext) {
+export async function allGlobalUsers(ctx: Ctx) {
   const response = await fetch(
     checkSlashesInUrl(env.WORKER_URL + "/api/global/users"),
     // we don't want to use API key when getting self
@@ -139,7 +124,7 @@ export async function allGlobalUsers(ctx: BBContext) {
   return checkResponse(response, "get users", { ctx })
 }
 
-export async function saveGlobalUser(ctx: BBContext) {
+export async function saveGlobalUser(ctx: Ctx) {
   const response = await fetch(
     checkSlashesInUrl(env.WORKER_URL + "/api/global/users"),
     // we don't want to use API key when getting self
@@ -148,7 +133,7 @@ export async function saveGlobalUser(ctx: BBContext) {
   return checkResponse(response, "save user", { ctx })
 }
 
-export async function deleteGlobalUser(ctx: BBContext) {
+export async function deleteGlobalUser(ctx: Ctx) {
   const response = await fetch(
     checkSlashesInUrl(
       env.WORKER_URL + `/api/global/users/${ctx.params.userId}`
@@ -159,7 +144,7 @@ export async function deleteGlobalUser(ctx: BBContext) {
   return checkResponse(response, "delete user", { ctx })
 }
 
-export async function readGlobalUser(ctx: BBContext): Promise<User> {
+export async function readGlobalUser(ctx: Ctx): Promise<User> {
   const response = await fetch(
     checkSlashesInUrl(
       env.WORKER_URL + `/api/global/users/${ctx.params.userId}`

packages/types/src/documents/global/userGroup.ts

@@ -7,6 +7,10 @@ export interface UserGroup extends Document {
   color: string
   users?: GroupUser[]
   roles?: UserGroupRoles
+  // same structure as users
+  builder?: {
+    apps: string[]
+  }
   createdAt?: number
   scimInfo?: {
     externalId: string

packages/worker/src/api/controllers/global/self.ts

@@ -48,7 +48,7 @@ export async function generateAPIKey(ctx: any) {
   } catch (err) {
     devInfo = { _id: id, userId }
   }
-  devInfo.apiKey = await apiKey
+  devInfo.apiKey = apiKey
   await db.put(devInfo)
   ctx.body = cleanupDevInfo(devInfo)
 }
@@ -63,7 +63,7 @@ export async function fetchAPIKey(ctx: any) {
     devInfo = {
       _id: id,
       userId: ctx.user._id,
-      apiKey: await newApiKey(),
+      apiKey: newApiKey(),
     }
     await db.put(devInfo)
   }

packages/worker/src/api/controllers/global/users.ts

@@ -25,11 +25,11 @@ import {
 import {
   accounts,
   cache,
+  ErrorCode,
   events,
   migrations,
-  tenancy,
   platform,
-  ErrorCode,
+  tenancy,
 } from "@budibase/backend-core"
 import { checkAnyUserExists } from "../../../utilities/users"
 import { isEmailConfigured } from "../../../utilities/email"
@@ -280,7 +280,7 @@ export const onboardUsers = async (ctx: Ctx<InviteUsersRequest>) => {
     let bulkCreateReponse = await userSdk.db.bulkCreate(users, [])
 
     // Apply temporary credentials
-    let createWithCredentials = {
+    ctx.body = {
       ...bulkCreateReponse,
       successful: bulkCreateReponse?.successful.map(user => {
         return {
@@ -290,8 +290,6 @@ export const onboardUsers = async (ctx: Ctx<InviteUsersRequest>) => {
       }),
       created: true,
     }
-
-    ctx.body = createWithCredentials
   } else {
     ctx.throw(400, "User onboarding failed")
   }

packages/worker/src/api/routes/global/tests/appBuilder.spec.ts

@@ -33,7 +33,9 @@ describe("/api/global/users/:userId/app/builder", () => {
         MOCK_APP_ID,
         400
       )
-      expect(resp.body.message).toContain("Feature not enabled")
+      expect(resp.body.message).toContain(
+        "appBuilders are not currently enabled"
+      )
     })
   })
 

packages/worker/src/api/routes/global/tests/scim.spec.ts

@@ -10,6 +10,8 @@ import {
 import { TestConfiguration } from "../../../../tests"
 import { events } from "@budibase/backend-core"
 
+jest.setTimeout(30000)
+
 mocks.licenses.useScimIntegration()
 
 describe("scim", () => {