Merge pull request #12807 from Budibase/feature/count-creators-in-groups

[Fixed] Calculate creators count when group role changes

packages/backend-core/src/index.ts

@@ -2,6 +2,7 @@ export * as configs from "./configs"
 export * as events from "./events"
 export * as migrations from "./migrations"
 export * as users from "./users"
+export * as userUtils from "./users/utils"
 export * as roles from "./security/roles"
 export * as permissions from "./security/permissions"
 export * as accounts from "./accounts"

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

@@ -251,7 +251,8 @@ export class UserDB {
     }
 
     const change = dbUser ? 0 : 1 // no change if there is existing user
-    const creatorsChange = isCreator(dbUser) !== isCreator(user) ? 1 : 0
+    const creatorsChange =
+      (await isCreator(dbUser)) !== (await isCreator(user)) ? 1 : 0
     return UserDB.quotas.addUsers(change, creatorsChange, async () => {
       await validateUniqueUser(email, tenantId)
 
@@ -335,7 +336,7 @@ export class UserDB {
       }
       newUser.userGroups = groups || []
       newUsers.push(newUser)
-      if (isCreator(newUser)) {
+      if (await isCreator(newUser)) {
         newCreators.push(newUser)
       }
     }
@@ -432,12 +433,16 @@ export class UserDB {
       _deleted: true,
     }))
     const dbResponse = await usersCore.bulkUpdateGlobalUsers(toDelete)
-    const creatorsToDelete = usersToDelete.filter(isCreator)
+
+    const creatorsEval = await Promise.all(usersToDelete.map(isCreator))
+    const creatorsToDeleteCount = creatorsEval.filter(
+      creator => !!creator
+    ).length
 
     for (let user of usersToDelete) {
       await bulkDeleteProcessing(user)
     }
-    await UserDB.quotas.removeUsers(toDelete.length, creatorsToDelete.length)
+    await UserDB.quotas.removeUsers(toDelete.length, creatorsToDeleteCount)
 
     // Build Response
     // index users by id
@@ -486,7 +491,7 @@ export class UserDB {
 
     await db.remove(userId, dbUser._rev)
 
-    const creatorsToDelete = isCreator(dbUser) ? 1 : 0
+    const creatorsToDelete = (await isCreator(dbUser)) ? 1 : 0
     await UserDB.quotas.removeUsers(1, creatorsToDelete)
     await eventHelpers.handleDeleteEvents(dbUser)
     await cache.user.invalidateUser(userId)

packages/backend-core/src/users/test/utils.spec.ts

@@ -0,0 +1,67 @@
+import { User, UserGroup } from "@budibase/types"
+import { generator, structures } from "../../../tests"
+import { DBTestConfiguration } from "../../../tests/extra"
+import { getGlobalDB } from "../../context"
+import { isCreator } from "../utils"
+
+const config = new DBTestConfiguration()
+
+describe("Users", () => {
+  it("User is a creator if it is configured as a global builder", async () => {
+    const user: User = structures.users.user({ builder: { global: true } })
+    expect(await isCreator(user)).toBe(true)
+  })
+
+  it("User is a creator if it is configured as a global admin", async () => {
+    const user: User = structures.users.user({ admin: { global: true } })
+    expect(await isCreator(user)).toBe(true)
+  })
+
+  it("User is a creator if it is configured with creator permission", async () => {
+    const user: User = structures.users.user({ builder: { creator: true } })
+    expect(await isCreator(user)).toBe(true)
+  })
+
+  it("User is a creator if it is a builder in some application", async () => {
+    const user: User = structures.users.user({ builder: { apps: ["app1"] } })
+    expect(await isCreator(user)).toBe(true)
+  })
+
+  it("User is a creator if it has CREATOR permission in some application", async () => {
+    const user: User = structures.users.user({ roles: { app1: "CREATOR" } })
+    expect(await isCreator(user)).toBe(true)
+  })
+
+  it("User is a creator if it has ADMIN permission in some application", async () => {
+    const user: User = structures.users.user({ roles: { app1: "ADMIN" } })
+    expect(await isCreator(user)).toBe(true)
+  })
+
+  it("User is a creator if it remains to a group with ADMIN permissions", async () => {
+    const usersInGroup = 10
+    const groupId = "gr_17abffe89e0b40268e755b952f101a59"
+    const group: UserGroup = {
+      ...structures.userGroups.userGroup(),
+      ...{ _id: groupId, roles: { app1: "ADMIN" } },
+    }
+    const users: User[] = []
+    for (const _ of Array.from({ length: usersInGroup })) {
+      const userId = `us_${generator.guid()}`
+      const user: User = structures.users.user({
+        _id: userId,
+        userGroups: [groupId],
+      })
+      users.push(user)
+    }
+
+    await config.doInTenant(async () => {
+      const db = getGlobalDB()
+      await db.put(group)
+      for (let user of users) {
+        await db.put(user)
+        const creator = await isCreator(user)
+        expect(creator).toBe(true)
+      }
+    })
+  })
+})

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

@@ -309,7 +309,8 @@ export async function getCreatorCount() {
   let creators = 0
   async function iterate(startPage?: string) {
     const page = await paginatedUsers({ bookmark: startPage })
-    creators += page.data.filter(isCreator).length
+    const creatorsEval = await Promise.all(page.data.map(isCreator))
+    creators += creatorsEval.filter(creator => !!creator).length
     if (page.hasNextPage) {
       await iterate(page.nextPage)
     }

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

@@ -1,4 +1,4 @@
-import { CloudAccount } from "@budibase/types"
+import { CloudAccount, ContextUser, User, UserGroup } from "@budibase/types"
 import * as accountSdk from "../accounts"
 import env from "../environment"
 import { getPlatformUser } from "./lookup"
@@ -6,17 +6,48 @@ import { EmailUnavailableError } from "../errors"
 import { getTenantId } from "../context"
 import { sdk } from "@budibase/shared-core"
 import { getAccountByTenantId } from "../accounts"
+import { BUILTIN_ROLE_IDS } from "../security/roles"
+import * as context from "../context"
 
 // extract from shared-core to make easily accessible from backend-core
 export const isBuilder = sdk.users.isBuilder
 export const isAdmin = sdk.users.isAdmin
-export const isCreator = sdk.users.isCreator
 export const isGlobalBuilder = sdk.users.isGlobalBuilder
 export const isAdminOrBuilder = sdk.users.isAdminOrBuilder
 export const hasAdminPermissions = sdk.users.hasAdminPermissions
 export const hasBuilderPermissions = sdk.users.hasBuilderPermissions
 export const hasAppBuilderPermissions = sdk.users.hasAppBuilderPermissions
 
+export async function isCreator(user?: User | ContextUser) {
+  const isCreatorByUserDefinition = sdk.users.isCreator(user)
+  if (!isCreatorByUserDefinition && user) {
+    return await isCreatorByGroupMembership(user)
+  }
+  return isCreatorByUserDefinition
+}
+
+async function isCreatorByGroupMembership(user?: User | ContextUser) {
+  const userGroups = user?.userGroups || []
+  if (userGroups.length > 0) {
+    const db = context.getGlobalDB()
+    const groups: UserGroup[] = []
+    for (let groupId of userGroups) {
+      try {
+        const group = await db.get<UserGroup>(groupId)
+        groups.push(group)
+      } catch (e: any) {
+        if (e.error !== "not_found") {
+          throw e
+        }
+      }
+    }
+    return groups.some(group =>
+      Object.values(group.roles || {}).includes(BUILTIN_ROLE_IDS.ADMIN)
+    )
+  }
+  return false
+}
+
 export async function validateUniqueUser(email: string, tenantId: string) {
   // check budibase users in other tenants
   if (env.MULTI_TENANCY) {

packages/pro

@@ -1 +1 @@
-Subproject commit ce7722ed4474718596b465dcfd49bef36cab2e42
+Subproject commit 0e4c5f95bda6af126a5cddeef01b8cf551f236be

packages/server/src/middleware/authorized.ts

@@ -34,7 +34,7 @@ const checkAuthorized = async (
   const isCreatorApi = permType === PermissionType.CREATOR
   const isBuilderApi = permType === PermissionType.BUILDER
   const isGlobalBuilder = users.isGlobalBuilder(ctx.user)
-  const isCreator = users.isCreator(ctx.user)
+  const isCreator = await users.isCreator(ctx.user)
   const isBuilder = appId
     ? users.isBuilder(ctx.user, appId)
     : users.hasBuilderPermissions(ctx.user)

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

@@ -84,7 +84,7 @@ describe("syncGlobalUsers", () => {
         await syncGlobalUsers()
 
         const metadata = await rawUserMetadata()
-        expect(metadata).toHaveLength(3)
+        expect(metadata).toHaveLength(2)
         expect(metadata).toContainEqual(
           expect.objectContaining({
             _id: db.generateUserMetadataID(user1._id!),
@@ -121,7 +121,7 @@ describe("syncGlobalUsers", () => {
         await syncGlobalUsers()
 
         const metadata = await rawUserMetadata()
-        expect(metadata).toHaveLength(0)
+        expect(metadata).toHaveLength(1) //ADMIN user created in test bootstrap still in the application
       })
     })
   })

packages/shared-core/src/sdk/documents/users.ts

@@ -70,7 +70,7 @@ export function hasAppCreatorPermissions(user?: User | ContextUser): boolean {
   return _.flow(
     _.get("roles"),
     _.values,
-    _.find(x => x === "CREATOR"),
+    _.find(x => ["CREATOR", "ADMIN"].includes(x)),
     x => !!x
   )(user)
 }

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

@@ -91,6 +91,9 @@ export async function getSelf(ctx: any) {
     id: userId,
   }
 
+  // Adjust creators quotas (prevents wrong creators count if user has changed the plan)
+  await groups.adjustGroupCreatorsQuotas()
+
   // get the main body of the user
   const user = await userSdk.db.getUser(userId)
   ctx.body = await groups.enrichUserRolesFromGroups(user)