Merge pull request #10153 from Budibase/budi-6803-delete-users-when-group-removed

BUDI-6803 - Remove users that should not be there anymore when syncGlobalUsers

packages/backend-core/tests/utilities/structures/index.ts

@@ -8,4 +8,5 @@ export * as plugins from "./plugins"
 export * as sso from "./sso"
 export * as tenant from "./tenants"
 export * as users from "./users"
+export * as userGroups from "./userGroups"
 export { generator } from "./generator"

packages/backend-core/tests/utilities/structures/userGroups.ts

@@ -0,0 +1,10 @@
+import { UserGroup } from "@budibase/types"
+import { generator } from "./generator"
+
+export function userGroup(): UserGroup {
+  return {
+    name: generator.word(),
+    icon: generator.word(),
+    color: generator.word(),
+  }
+}

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

@@ -0,0 +1,159 @@
+import { db, roles } from "@budibase/backend-core"
+import { structures } from "@budibase/backend-core/tests"
+import { sdk as proSdk } from "@budibase/pro"
+
+import TestConfiguration from "../../../tests/utilities/TestConfiguration"
+import { rawUserMetadata, syncGlobalUsers } from "../utils"
+
+describe("syncGlobalUsers", () => {
+  const config = new TestConfiguration()
+
+  beforeEach(async () => {
+    await config.init()
+  })
+
+  afterAll(config.end)
+
+  it("the default user is synced", async () => {
+    await config.doInContext(config.appId, async () => {
+      await syncGlobalUsers()
+
+      const metadata = await rawUserMetadata()
+      expect(metadata).toHaveLength(1)
+      expect(metadata).toEqual([
+        expect.objectContaining({
+          _id: db.generateUserMetadataID(config.user._id),
+        }),
+      ])
+    })
+  })
+
+  it("admin and builders users are synced", async () => {
+    const user1 = await config.createUser({ admin: true })
+    const user2 = await config.createUser({ admin: false, builder: true })
+    await config.doInContext(config.appId, async () => {
+      expect(await rawUserMetadata()).toHaveLength(1)
+      await syncGlobalUsers()
+
+      const metadata = await rawUserMetadata()
+      expect(metadata).toHaveLength(3)
+      expect(metadata).toContainEqual(
+        expect.objectContaining({
+          _id: db.generateUserMetadataID(user1._id),
+        })
+      )
+      expect(metadata).toContainEqual(
+        expect.objectContaining({
+          _id: db.generateUserMetadataID(user2._id),
+        })
+      )
+    })
+  })
+
+  it("app users are not synced if not specified", async () => {
+    const user = await config.createUser({ admin: false, builder: false })
+    await config.doInContext(config.appId, async () => {
+      await syncGlobalUsers()
+
+      const metadata = await rawUserMetadata()
+      expect(metadata).toHaveLength(1)
+      expect(metadata).not.toContainEqual(
+        expect.objectContaining({
+          _id: db.generateUserMetadataID(user._id),
+        })
+      )
+    })
+  })
+
+  it("app users are added when group is assigned to app", 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.addUsers(group.id, [user1._id, user2._id])
+
+      await config.doInContext(config.appId, async () => {
+        await syncGlobalUsers()
+        expect(await rawUserMetadata()).toHaveLength(1)
+
+        await proSdk.groups.updateGroupApps(group.id, {
+          appsToAdd: [
+            { appId: config.prodAppId!, roleId: roles.BUILTIN_ROLE_IDS.BASIC },
+          ],
+        })
+        await syncGlobalUsers()
+
+        const metadata = await rawUserMetadata()
+        expect(metadata).toHaveLength(3)
+        expect(metadata).toContainEqual(
+          expect.objectContaining({
+            _id: db.generateUserMetadataID(user1._id),
+          })
+        )
+        expect(metadata).toContainEqual(
+          expect.objectContaining({
+            _id: db.generateUserMetadataID(user2._id),
+          })
+        )
+      })
+    })
+  })
+
+  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.updateGroupApps(group.id, {
+          appsToRemove: [{ appId: config.prodAppId! }],
+        })
+        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),
+          })
+        )
+      })
+    })
+  })
+})

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

@@ -6,25 +6,33 @@ import {
   InternalTables,
 } from "../../db/utils"
 import { isEqual } from "lodash"
+import { ContextUser, UserMetadata } from "@budibase/types"
 
-export function combineMetadataAndUser(user: any, metadata: any) {
+export function combineMetadataAndUser(
+  user: ContextUser,
+  metadata: UserMetadata | UserMetadata[]
+) {
+  const metadataId = generateUserMetadataID(user._id!)
+  const found = Array.isArray(metadata)
+    ? metadata.find(doc => doc._id === metadataId)
+    : metadata
   // skip users with no access
   if (
     user.roleId == null ||
     user.roleId === rolesCore.BUILTIN_ROLE_IDS.PUBLIC
   ) {
+    // If it exists and it should not, we must remove it
+    if (found?._id) {
+      return { ...found, _deleted: true }
+    }
     return null
   }
   delete user._rev
-  const metadataId = generateUserMetadataID(user._id)
   const newDoc = {
     ...user,
     _id: metadataId,
     tableId: InternalTables.USER_METADATA,
   }
-  const found = Array.isArray(metadata)
-    ? metadata.find(doc => doc._id === metadataId)
-    : metadata
   // copy rev over for the purposes of equality check
   if (found) {
     newDoc._rev = found._rev
@@ -58,7 +66,7 @@ export async function syncGlobalUsers() {
   ])
   const toWrite = []
   for (let user of users) {
-    const combined = await combineMetadataAndUser(user, metadata)
+    const combined = combineMetadataAndUser(user, metadata)
     if (combined) {
       toWrite.push(combined)
     }

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

@@ -47,6 +47,7 @@ import {
   SourceName,
   Table,
   SearchFilters,
+  UserRoles,
 } from "@budibase/types"
 
 type DefaultUserValues = {
@@ -277,7 +278,7 @@ class TestConfiguration {
       email?: string
       builder?: boolean
       admin?: boolean
-      roles?: any
+      roles?: UserRoles
     } = {}
   ) {
     let { id, firstName, lastName, email, builder, admin, roles } = user