Some fixes post testing.

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

@@ -10,6 +10,7 @@ import { getAccountByTenantId } from "../accounts"
 // 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 isGlobalBuilder = sdk.users.isGlobalBuilder
 export const isAdminOrBuilder = sdk.users.isAdminOrBuilder
 export const hasAdminPermissions = sdk.users.hasAdminPermissions
 export const hasBuilderPermissions = sdk.users.hasBuilderPermissions

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

@@ -14,6 +14,10 @@ export function isBuilder(user: User | ContextUser, appId?: string) {
   return false
 }
 
+export function isGlobalBuilder(user: User | ContextUser) {
+  return (isBuilder(user) && !hasAppBuilderPermissions(user)) || isAdmin(user)
+}
+
 // alias for hasAdminPermission, currently do the same thing
 // in future whether someone has admin permissions and whether they are
 // an admin for a specific resource could be separated

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

@@ -447,17 +447,20 @@ export const grantAppBuilder = async (ctx: Ctx) => {
 export const addAppBuilder = async (ctx: Ctx) => {
   const { userId, appId } = ctx.params
   const user = await userSdk.db.getUser(userId)
-  if (!user.builder?.global || user.admin?.global) {
+  if (!user.builder?.appBuilder && !userSdk.core.isGlobalBuilder(user)) {
-    ctx.body = { message: "User already admin - no permissions updated." }
-    return
-  }
-  if (!user.builder?.appBuilder) {
     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
+  }
   const prodAppId = dbCore.getProdAppID(appId)
+  if (!user.builder) {
+    user.builder = {}
+  }
   if (!user.builder.apps) {
     user.builder.apps = []
   }
@@ -469,19 +472,19 @@ 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?.global || user.admin?.global) {
+  if (!user.builder?.appBuilder && !userSdk.core.isGlobalBuilder(user)) {
-    ctx.body = { message: "User already admin - no permissions removed." }
-    return
-  }
-  if (!user.builder?.appBuilder) {
     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
+  }
   const prodAppId = dbCore.getProdAppID(appId)
   const indexOf = user.builder?.apps?.indexOf(prodAppId)
-  if (indexOf && indexOf !== -1) {
+  if (user.builder && indexOf != undefined && indexOf !== -1) {
     user.builder.apps = user.builder.apps!.splice(indexOf, 1)
   }
   await userSdk.db.save(user, { hashPassword: false })

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

@@ -48,7 +48,7 @@ describe("/api/global/users/:userId/app/builder", () => {
       await config.api.users.grantBuilderToApp(user._id!, MOCK_APP_ID)
       const updated = await getUser(user._id!)
       expect(updated.builder?.appBuilder).toBe(true)
-      expect(updated.builder?.apps).toBe([MOCK_APP_ID])
+      expect(updated.builder?.apps![0]).toBe(MOCK_APP_ID)
     })
   })
 
@@ -57,10 +57,10 @@ describe("/api/global/users/:userId/app/builder", () => {
       const user = await grantAppBuilder()
       await config.api.users.grantBuilderToApp(user._id!, MOCK_APP_ID)
       let updated = await getUser(user._id!)
-      expect(updated.builder?.apps).toBe([MOCK_APP_ID])
+      expect(updated.builder?.apps![0]).toBe(MOCK_APP_ID)
       await config.api.users.revokeBuilderToApp(user._id!, MOCK_APP_ID)
       updated = await getUser(user._id!)
-      expect(updated.builder?.apps).toBe([])
+      expect(updated.builder?.apps!.length).toBe(0)
     })
   })
 })

packages/worker/src/tests/api/users.ts

@@ -163,8 +163,7 @@ export class UserAPI extends TestAPI {
 
   revokeBuilderToApp = (
     userId: string,
-    appId: string,
+    appId: string
-    statusCode: number = 200
   ) => {
     return this.request
       .delete(`/api/global/users/${userId}/app/${appId}/builder`)