Merge branch 'fix/BUDI-6754' of github.com:Budibase/budibase into fix/BUDI-6754

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

@@ -42,7 +42,11 @@ async function removeDeprecated(db: Database, viewName: ViewName) {
   }
 }
 
-export async function createView(db: any, viewJs: string, viewName: string) {
+export async function createView(
+  db: any,
+  viewJs: string,
+  viewName: string
+): Promise<void> {
   let designDoc
   try {
     designDoc = (await db.get(DESIGN_DB)) as DesignDocument
@@ -57,7 +61,15 @@ export async function createView(db: any, viewJs: string, viewName: string) {
     ...designDoc.views,
     [viewName]: view,
   }
-  await db.put(designDoc)
+  try {
+    await db.put(designDoc)
+  } catch (err: any) {
+    if (err.status === 409) {
+      return await createView(db, viewJs, viewName)
+    } else {
+      throw err
+    }
+  }
 }
 
 export const createNewUserEmailView = async () => {
@@ -135,6 +147,10 @@ export const queryView = async <T>(
       await removeDeprecated(db, viewName)
       await createFunc()
       return queryView(viewName, params, db, createFunc, opts)
+    } else if (err.status === 409) {
+      // can happen when multiple queries occur at once, view couldn't be created
+      // other design docs being updated, re-run
+      return queryView(viewName, params, db, createFunc, opts)
     } else {
       throw err
     }

packages/server/src/api/controllers/row/internal.ts

@@ -30,7 +30,6 @@ import { finaliseRow, updateRelatedFormula } from "./staticFormula"
 import { csv, json, jsonWithSchema, Format } from "../view/exporters"
 import { apiFileReturn } from "../../../utilities/fileSystem"
 import {
-  Ctx,
   UserCtx,
   Database,
   LinkDocumentValue,
@@ -72,7 +71,7 @@ async function getView(db: Database, viewName: string) {
   return viewInfo
 }
 
-async function getRawTableData(ctx: Ctx, db: Database, tableId: string) {
+async function getRawTableData(ctx: UserCtx, db: Database, tableId: string) {
   let rows
   if (tableId === InternalTables.USER_METADATA) {
     await userController.fetchMetadata(ctx)
@@ -188,7 +187,7 @@ export async function save(ctx: UserCtx) {
   })
 }
 
-export async function fetchView(ctx: Ctx) {
+export async function fetchView(ctx: UserCtx) {
   const viewName = decodeURIComponent(ctx.params.viewName)
 
   // if this is a table view being looked for just transfer to that
@@ -255,7 +254,7 @@ export async function fetchView(ctx: Ctx) {
   return rows
 }
 
-export async function fetch(ctx: Ctx) {
+export async function fetch(ctx: UserCtx) {
   const db = context.getAppDB()
 
   const tableId = ctx.params.tableId
@@ -264,7 +263,7 @@ export async function fetch(ctx: Ctx) {
   return outputProcessing(table, rows)
 }
 
-export async function find(ctx: Ctx) {
+export async function find(ctx: UserCtx) {
   const db = dbCore.getDB(ctx.appId)
   const table = await db.get(ctx.params.tableId)
   let row = await utils.findRow(ctx, ctx.params.tableId, ctx.params.rowId)
@@ -272,7 +271,7 @@ export async function find(ctx: Ctx) {
   return row
 }
 
-export async function destroy(ctx: Ctx) {
+export async function destroy(ctx: UserCtx) {
   const db = context.getAppDB()
   const { _id } = ctx.request.body
   let row = await db.get(_id)
@@ -308,7 +307,7 @@ export async function destroy(ctx: Ctx) {
   return { response, row }
 }
 
-export async function bulkDestroy(ctx: Ctx) {
+export async function bulkDestroy(ctx: UserCtx) {
   const db = context.getAppDB()
   const tableId = ctx.params.tableId
   const table = await db.get(tableId)
@@ -347,7 +346,7 @@ export async function bulkDestroy(ctx: Ctx) {
   return { response: { ok: true }, rows: processedRows }
 }
 
-export async function search(ctx: Ctx) {
+export async function search(ctx: UserCtx) {
   // Fetch the whole table when running in cypress, as search doesn't work
   if (!env.COUCH_DB_URL && env.isCypress()) {
     return { rows: await fetch(ctx) }
@@ -387,7 +386,7 @@ export async function search(ctx: Ctx) {
   return response
 }
 
-export async function exportRows(ctx: Ctx) {
+export async function exportRows(ctx: UserCtx) {
   const db = context.getAppDB()
   const table = await db.get(ctx.params.tableId)
   const rowIds = ctx.request.body.rows
@@ -439,7 +438,7 @@ export async function exportRows(ctx: Ctx) {
   }
 }
 
-export async function fetchEnrichedRow(ctx: Ctx) {
+export async function fetchEnrichedRow(ctx: UserCtx) {
   const db = context.getAppDB()
   const tableId = ctx.params.tableId
   const rowId = ctx.params.rowId

packages/server/src/api/controllers/row/utils.ts

@@ -5,7 +5,7 @@ import { context } from "@budibase/backend-core"
 import { makeExternalQuery } from "../../../integrations/base/query"
 import { Row, Table } from "@budibase/types"
 import { Format } from "../view/exporters"
-import { Ctx } from "@budibase/types"
+import { UserCtx } from "@budibase/types"
 import sdk from "../../../sdk"
 const validateJs = require("validate.js")
 const { cloneDeep } = require("lodash/fp")
@@ -26,7 +26,7 @@ export async function getDatasourceAndQuery(json: any) {
   return makeExternalQuery(datasource, json)
 }
 
-export async function findRow(ctx: Ctx, tableId: string, rowId: string) {
+export async function findRow(ctx: UserCtx, tableId: string, rowId: string) {
   const db = context.getAppDB()
   let row
   // TODO remove special user case in future

packages/server/src/api/routes/tests/user.spec.js

@@ -205,41 +205,4 @@ describe("/users", () => {
       expect(res.body.message).toEqual("Flag set successfully")
     })
   })
-
-  describe("syncUser", () => {
-    it("should sync the user", async () => {
-      let user = await config.createUser()
-      await config.createApp("New App")
-      let res = await request
-        .post(`/api/users/metadata/sync/${user._id}`)
-        .set(config.defaultHeaders())
-        .expect(200)
-        .expect("Content-Type", /json/)
-      expect(res.body.message).toEqual("User synced.")
-    })
-
-    it("should sync the user when a previous user is specified", async () => {
-      const app1 = await config.createApp("App 1")
-      const app2 = await config.createApp("App 2")
-
-      let user = await config.createUser({
-        builder: false,
-        admin: true,
-        roles: { [app1.appId]: "ADMIN" },
-      })
-      let res = await request
-        .post(`/api/users/metadata/sync/${user._id}`)
-        .set(config.defaultHeaders())
-        .send({
-          previousUser: {
-            ...user,
-            roles: { ...user.roles, [app2.appId]: "BASIC" },
-          },
-        })
-        .expect(200)
-        .expect("Content-Type", /json/)
-
-      expect(res.body.message).toEqual("User synced.")
-    })
-  })
 })

packages/server/src/sdk/app/applications/sync.ts

@@ -7,15 +7,19 @@ import {
   logging,
   roles,
 } from "@budibase/backend-core"
-import { User, ContextUser } from "@budibase/types"
+import { User, ContextUser, UserGroup } from "@budibase/types"
 import { sdk as proSdk } from "@budibase/pro"
 import sdk from "../../"
-import { getGlobalUsers, updateAppRole } from "../../../utilities/global"
+import { getGlobalUsers, processUser } from "../../../utilities/global"
 import { generateUserMetadataID, InternalTables } from "../../../db/utils"
 
 type DeletedUser = { _id: string; deleted: boolean }
 
-async function syncUsersToApp(appId: string, users: (User | DeletedUser)[]) {
+async function syncUsersToApp(
+  appId: string,
+  users: (User | DeletedUser)[],
+  groups: UserGroup[]
+) {
   if (!(await dbCore.dbExists(appId))) {
     return
   }
@@ -31,7 +35,7 @@ async function syncUsersToApp(appId: string, users: (User | DeletedUser)[]) {
 
       // make sure role is correct
       if (!deletedUser) {
-        ctxUser = updateAppRole(ctxUser, { appId })
+        ctxUser = await processUser(ctxUser, { appId, groups })
       }
       let roleId = ctxUser.roleId
       if (roleId === roles.BUILTIN_ROLE_IDS.PUBLIC) {
@@ -80,7 +84,10 @@ async function syncUsersToApp(appId: string, users: (User | DeletedUser)[]) {
 
 async function syncUsersToAllApps(userIds: string[]) {
   // list of users, if one has been deleted it will be undefined in array
-  const users = (await getGlobalUsers(userIds)) as User[]
+  const users = (await getGlobalUsers(userIds, {
+    noProcessing: true,
+  })) as User[]
+  const groups = await proSdk.groups.fetch()
   const finalUsers: (User | DeletedUser)[] = []
   for (let userId of userIds) {
     const user = users.find(user => user._id === userId)
@@ -95,7 +102,7 @@ async function syncUsersToAllApps(userIds: string[]) {
   for (let devAppId of devAppIds) {
     const prodAppId = dbCore.getProdAppID(devAppId)
     for (let appId of [prodAppId, devAppId]) {
-      promises.push(syncUsersToApp(appId, finalUsers))
+      promises.push(syncUsersToApp(appId, finalUsers, groups))
     }
   }
   const resp = await Promise.allSettled(promises)
@@ -106,24 +113,32 @@ async function syncUsersToAllApps(userIds: string[]) {
   }
 }
 
-export function initUserGroupSync(updateCb?: () => void) {
+export function initUserGroupSync(updateCb?: (docId: string) => void) {
   const types = [constants.DocumentType.USER, constants.DocumentType.GROUP]
   docUpdates.process(types, async update => {
-    const docId = update.id
-    const isGroup = docId.startsWith(constants.DocumentType.GROUP)
-    let userIds: string[]
-    if (isGroup) {
-      const group = await proSdk.groups.get(docId)
-      userIds = group.users?.map(user => user._id) || []
-    } else {
-      userIds = [docId]
-    }
-    if (userIds.length > 0) {
-      await syncUsersToAllApps(userIds)
-    }
-    // used to tracking when updates have occurred
-    if (updateCb) {
-      updateCb()
+    try {
+      const docId = update.id
+      const isGroup = docId.startsWith(constants.DocumentType.GROUP)
+      let userIds: string[]
+      if (isGroup) {
+        const group = await proSdk.groups.get(docId)
+        userIds = group.users?.map(user => user._id) || []
+      } else {
+        userIds = [docId]
+      }
+      if (userIds.length > 0) {
+        await syncUsersToAllApps(userIds)
+      }
+      if (updateCb) {
+        updateCb(docId)
+      }
+    } catch (err: any) {
+      // if something not found - no changes to perform
+      if (err?.status === 404) {
+        return
+      } else {
+        logging.logAlert("Failed to perform user/group app sync", err)
+      }
     }
   })
 }

packages/server/src/sdk/app/applications/tests/sync.spec.ts

@@ -1,26 +1,32 @@
 import TestConfiguration from "../../../../tests/utilities/TestConfiguration"
-import { events, context, roles, db as dbCore } from "@budibase/backend-core"
+import { events, context, roles, constants } from "@budibase/backend-core"
 import { initUserGroupSync } from "../sync"
 import { rawUserMetadata } from "../../../users/utils"
 import EventEmitter from "events"
-import { UserMetadata, UserRoles } from "@budibase/types"
+import { UserGroup, UserMetadata, UserRoles, User } from "@budibase/types"
 
 const config = new TestConfiguration()
-let app
+let app, group: UserGroup, groupUser: User
 const ROLE_ID = roles.BUILTIN_ROLE_IDS.BASIC
 
 const emitter = new EventEmitter()
 
-function updateCb() {
-  emitter.emit("update")
+function updateCb(docId: string) {
+  const isGroup = docId.startsWith(constants.DocumentType.GROUP)
+  if (isGroup) {
+    emitter.emit("update-group")
+  } else {
+    emitter.emit("update-user")
+  }
 }
 
-function waitForUpdate() {
+function waitForUpdate(opts: { group?: boolean }) {
   return new Promise<void>((resolve, reject) => {
     const timeout = setTimeout(() => {
       reject()
     }, 5000)
-    emitter.on("update", () => {
+    const event = opts?.group ? "update-group" : "update-user"
+    emitter.on(event, () => {
       clearTimeout(timeout)
       resolve()
     })
@@ -32,30 +38,99 @@ beforeAll(async () => {
   initUserGroupSync(updateCb)
 })
 
-async function createUser(email: string, roles: UserRoles, appId?: string) {
-  const user = await config.createUser({ email, roles })
-  await context.doInContext(appId || config.appId!, async () => {
+async function createUser(email: string, roles: UserRoles, builder?: boolean) {
+  const user = await config.createUser({
+    email,
+    roles,
+    builder: builder || false,
+    admin: false,
+  })
+  await context.doInContext(config.appId!, async () => {
     await events.user.created(user)
   })
+  return user
+}
+
+async function removeUserRole(user: User) {
+  const final = await config.globalUser({
+    ...user,
+    id: user._id,
+    roles: {},
+    builder: false,
+    admin: false,
+  })
+  await context.doInContext(config.appId!, async () => {
+    await events.user.updated(final)
+  })
 }
 
-async function getUserMetadata(appId?: string): Promise<UserMetadata[]> {
-  return context.doInContext(appId || config.appId!, async () => {
+async function createGroupAndUser(email: string) {
+  groupUser = await config.createUser({
+    email,
+    roles: {},
+    builder: false,
+    admin: false,
+  })
+  group = await config.createGroup()
+  await config.addUserToGroup(group._id!, groupUser._id!)
+}
+
+async function removeUserFromGroup() {
+  await config.removeUserFromGroup(group._id!, groupUser._id!)
+  return context.doInContext(config.appId!, async () => {
+    await events.user.updated(groupUser)
+  })
+}
+
+async function getUserMetadata(): Promise<UserMetadata[]> {
+  return context.doInContext(config.appId!, async () => {
     return await rawUserMetadata()
   })
 }
 
-function buildRoles(appId?: string) {
-  const prodAppId = dbCore.getProdAppID(appId || config.appId!)
-  return { [prodAppId]: ROLE_ID }
+function buildRoles() {
+  return { [config.prodAppId!]: ROLE_ID }
 }
 
 describe("app user/group sync", () => {
-  it("should be able to sync a new user", async () => {
-    const email = "test@test.com"
-    await createUser(email, buildRoles())
-    await waitForUpdate()
+  const groupEmail = "test2@test.com",
+    normalEmail = "test@test.com"
+  async function checkEmail(
+    email: string,
+    opts?: { group?: boolean; notFound?: boolean }
+  ) {
+    await waitForUpdate(opts || {})
     const metadata = await getUserMetadata()
-    expect(metadata.find(data => data.email === email)).toBeDefined()
+    const found = metadata.find(data => data.email === email)
+    if (opts?.notFound) {
+      expect(found).toBeUndefined()
+    } else {
+      expect(found).toBeDefined()
+    }
+  }
+
+  it("should be able to sync a new user, add then remove", async () => {
+    const user = await createUser(normalEmail, buildRoles())
+    await checkEmail(normalEmail)
+    await removeUserRole(user)
+    await checkEmail(normalEmail, { notFound: true })
+  })
+
+  it("should be able to sync a group", async () => {
+    await createGroupAndUser(groupEmail)
+    await checkEmail(groupEmail, { group: true })
+  })
+
+  it("should be able to remove user from group", async () => {
+    if (!group) {
+      await createGroupAndUser(groupEmail)
+    }
+    await removeUserFromGroup()
+    await checkEmail(groupEmail, { notFound: true })
+  })
+
+  it("should be able to handle builder users", async () => {
+    await createUser("test3@test.com", {}, true)
+    await checkEmail("test3@test.com")
   })
 })

packages/server/src/sdk/users/tests/utils.spec.ts

@@ -121,38 +121,7 @@ describe("syncGlobalUsers", () => {
         await syncGlobalUsers()
 
         const metadata = await rawUserMetadata()
-        expect(metadata).toHaveLength(1)
-      })
-    })
-  })
-
-  it("app users are removed when app is removed from user group", async () => {
-    await config.doInTenant(async () => {
-      const group = await proSdk.groups.save(structures.userGroups.userGroup())
-      const user1 = await config.createUser({ admin: false, builder: false })
-      const user2 = await config.createUser({ admin: false, builder: false })
-      await proSdk.groups.updateGroupApps(group.id, {
-        appsToAdd: [
-          { appId: config.prodAppId!, roleId: roles.BUILTIN_ROLE_IDS.BASIC },
-        ],
-      })
-      await proSdk.groups.addUsers(group.id, [user1._id, user2._id])
-
-      await config.doInContext(config.appId, async () => {
-        await syncGlobalUsers()
-        expect(await rawUserMetadata()).toHaveLength(3)
-
-        await proSdk.groups.removeUsers(group.id, [user1._id])
-        await syncGlobalUsers()
-
-        const metadata = await rawUserMetadata()
-        expect(metadata).toHaveLength(2)
-
-        expect(metadata).not.toContainEqual(
-          expect.objectContaining({
-            _id: db.generateUserMetadataID(user1._id),
-          })
-        )
+        expect(metadata).toHaveLength(0)
       })
     })
   })

packages/server/src/tests/utilities/TestConfiguration.ts

@@ -49,6 +49,7 @@ import {
   SearchFilters,
   UserRoles,
 } from "@budibase/types"
+import { BUILTIN_ROLE_IDS } from "@budibase/backend-core/src/security/roles"
 
 type DefaultUserValues = {
   globalUserId: string
@@ -306,6 +307,33 @@ class TestConfiguration {
     }
   }
 
+  async createGroup(roleId: string = BUILTIN_ROLE_IDS.BASIC) {
+    return context.doInTenant(this.tenantId!, async () => {
+      const baseGroup = structures.userGroups.userGroup()
+      baseGroup.roles = {
+        [this.prodAppId]: roleId,
+      }
+      const { id, rev } = await pro.sdk.groups.save(baseGroup)
+      return {
+        _id: id,
+        _rev: rev,
+        ...baseGroup,
+      }
+    })
+  }
+
+  async addUserToGroup(groupId: string, userId: string) {
+    return context.doInTenant(this.tenantId!, async () => {
+      await pro.sdk.groups.addUsers(groupId, [userId])
+    })
+  }
+
+  async removeUserFromGroup(groupId: string, userId: string) {
+    return context.doInTenant(this.tenantId!, async () => {
+      await pro.sdk.groups.removeUsers(groupId, [userId])
+    })
+  }
+
   async login({ roleId, userId, builder, prodApp = false }: any = {}) {
     const appId = prodApp ? this.prodAppId : this.appId
     return context.doInAppContext(appId, async () => {

packages/server/src/utilities/global.ts

@@ -9,6 +9,7 @@ import {
 import env from "../environment"
 import { groups } from "@budibase/pro"
 import { UserCtx, ContextUser, User, UserGroup } from "@budibase/types"
+import { global } from "yargs"
 
 export function updateAppRole(
   user: ContextUser,
@@ -16,7 +17,7 @@ export function updateAppRole(
 ) {
   appId = appId || context.getAppId()
 
-  if (!user || !user.roles) {
+  if (!user || (!user.roles && !user.userGroups)) {
     return user
   }
   // if in an multi-tenancy environment make sure roles are never updated
@@ -27,7 +28,7 @@ export function updateAppRole(
     return user
   }
   // always use the deployed app
-  if (appId) {
+  if (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)
@@ -60,7 +61,7 @@ async function checkGroupRoles(
   return user
 }
 
-async function processUser(
+export async function processUser(
   user: ContextUser,
   opts: { appId?: string; groups?: UserGroup[] } = {}
 ) {
@@ -94,10 +95,12 @@ export async function getGlobalUser(userId: string) {
   return processUser(user, { appId })
 }
 
-export async function getGlobalUsers(userIds?: string[]) {
+export async function getGlobalUsers(
+  userIds?: string[],
+  opts?: { noProcessing?: boolean }
+) {
   const appId = context.getAppId()
   const db = tenancy.getGlobalDB()
-  const allGroups = await groups.fetch()
   let globalUsers
   if (userIds) {
     globalUsers = (await db.allDocs(getMultiIDParams(userIds))).rows.map(
@@ -123,11 +126,16 @@ export async function getGlobalUsers(userIds?: string[]) {
     return globalUsers
   }
 
-  // pass in the groups, meaning we don't actually need to retrieve them for
-  // each user individually
-  return Promise.all(
-    globalUsers.map(user => processUser(user, { groups: allGroups }))
-  )
+  if (opts?.noProcessing) {
+    return globalUsers
+  } else {
+    // pass in the groups, meaning we don't actually need to retrieve them for
+    // each user individually
+    const allGroups = await groups.fetch()
+    return Promise.all(
+      globalUsers.map(user => processUser(user, { groups: allGroups }))
+    )
+  }
 }
 
 export async function getGlobalUsersFromMetadata(users: ContextUser[]) {

qa-core/src/internal-api/tests/applications/publish.spec.ts

@@ -36,7 +36,7 @@ describe("Internal API - Application creation, update, publish and delete", () =
 
     const [syncResponse, sync] = await config.api.apps.sync(app.appId!)
     expect(sync).toEqual({
-      message: "App sync not required, app not deployed.",
+      message: "App sync completed successfully.",
     })
   })