diff --git a/packages/backend-core/src/users/utils.ts b/packages/backend-core/src/users/utils.ts index b6bf3f0abb..af0e8e10c7 100644 --- a/packages/backend-core/src/users/utils.ts +++ b/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 diff --git a/packages/shared-core/src/sdk/documents/users.ts b/packages/shared-core/src/sdk/documents/users.ts index 1a9314f731..92379a03ba 100644 --- a/packages/shared-core/src/sdk/documents/users.ts +++ b/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 diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index 5984f39ef8..d1e66b4ac1 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/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) { - ctx.body = { message: "User already admin - no permissions updated." } - return - } - if (!user.builder?.appBuilder) { + if (!user.builder?.appBuilder && !userSdk.core.isGlobalBuilder(user)) { 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) { - ctx.body = { message: "User already admin - no permissions removed." } - return - } - if (!user.builder?.appBuilder) { + if (!user.builder?.appBuilder && !userSdk.core.isGlobalBuilder(user)) { 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 }) diff --git a/packages/worker/src/api/routes/global/tests/appBuilder.spec.ts b/packages/worker/src/api/routes/global/tests/appBuilder.spec.ts index 138ebbc595..83bc401759 100644 --- a/packages/worker/src/api/routes/global/tests/appBuilder.spec.ts +++ b/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) }) }) }) diff --git a/packages/worker/src/tests/api/users.ts b/packages/worker/src/tests/api/users.ts index 605ac79416..bafc157e69 100644 --- a/packages/worker/src/tests/api/users.ts +++ b/packages/worker/src/tests/api/users.ts @@ -163,8 +163,7 @@ export class UserAPI extends TestAPI { revokeBuilderToApp = ( userId: string, - appId: string, - statusCode: number = 200 + appId: string ) => { return this.request .delete(`/api/global/users/${userId}/app/${appId}/builder`)