diff --git a/packages/server/src/api/controllers/public/mapping/users.ts b/packages/server/src/api/controllers/public/mapping/users.ts index db08b16552..2a158bede9 100644 --- a/packages/server/src/api/controllers/public/mapping/users.ts +++ b/packages/server/src/api/controllers/public/mapping/users.ts @@ -15,10 +15,15 @@ function user(body: any): User { } } -function mapUser(ctx: any): { data: User } { - return { +function mapUser(ctx: any) { + const body: { data: User; message?: string } = { data: user(ctx.body), } + if (ctx.extra?.message) { + body.message = ctx.extra.message + delete ctx.extra + } + return body } function mapUsers(ctx: any): { data: User[] } { diff --git a/packages/server/src/api/controllers/public/users.ts b/packages/server/src/api/controllers/public/users.ts index bb6fc3a6e7..4265c7ac22 100644 --- a/packages/server/src/api/controllers/public/users.ts +++ b/packages/server/src/api/controllers/public/users.ts @@ -10,6 +10,32 @@ import { search as stringSearch } from "./utils" import { UserCtx, User } from "@budibase/types" import { Next } from "koa" import { sdk } from "@budibase/pro" +import { isEqual, cloneDeep } from "lodash" + +function rolesRemoved(base: User, ctx: UserCtx) { + return ( + !isEqual(base.builder, ctx.request.body.builder) || + !isEqual(base.admin, ctx.request.body.admin) || + !isEqual(base.roles, ctx.request.body.roles) + ) +} + +const NO_ROLES_MSG = + "Roles/admin/builder can only be set on business/enterprise licenses - input ignored." + +async function createUpdateResponse(ctx: UserCtx, user?: User) { + const base = cloneDeep(ctx.request.body) + ctx = await sdk.publicApi.users.roleCheck(ctx, user) + // check the ctx before any updates to it + const removed = rolesRemoved(base, ctx) + ctx = publicApiUserFix(ctx) + const response = await saveGlobalUser(ctx) + ctx.body = await getUser(ctx, response._id) + if (removed) { + ctx.extra = { message: NO_ROLES_MSG } + } + return ctx +} function isLoggedInUser(ctx: UserCtx, user: User) { const loggedInId = ctx.user?._id @@ -35,9 +61,7 @@ export async function search(ctx: UserCtx, next: Next) { } export async function create(ctx: UserCtx, next: Next) { - ctx = publicApiUserFix(await sdk.publicApi.users.roleCheck(ctx)) - const response = await saveGlobalUser(ctx) - ctx.body = await getUser(ctx, response._id) + await createUpdateResponse(ctx) await next() } @@ -52,9 +76,7 @@ export async function update(ctx: UserCtx, next: Next) { ...ctx.request.body, _rev: user._rev, } - ctx = publicApiUserFix(await sdk.publicApi.users.roleCheck(ctx, user)) - const response = await saveGlobalUser(ctx) - ctx.body = await getUser(ctx, response._id) + await createUpdateResponse(ctx, user) await next() } diff --git a/packages/server/src/api/routes/public/tests/users.spec.ts b/packages/server/src/api/routes/public/tests/users.spec.ts index c81acca1df..7856409bec 100644 --- a/packages/server/src/api/routes/public/tests/users.spec.ts +++ b/packages/server/src/api/routes/public/tests/users.spec.ts @@ -68,6 +68,7 @@ describe("no user role update in free", () => { }) expect(res.status).toBe(200) expect(res.body.data.roles["app_a"]).toBeUndefined() + expect(res.body.message).toBeDefined() }) it("should not allow 'admin' to be updated", async () => { @@ -77,6 +78,7 @@ describe("no user role update in free", () => { }) expect(res.status).toBe(200) expect(res.body.data.admin).toBeUndefined() + expect(res.body.message).toBeDefined() }) it("should not allow 'builder' to be updated", async () => { @@ -86,6 +88,7 @@ describe("no user role update in free", () => { }) expect(res.status).toBe(200) expect(res.body.data.builder).toBeUndefined() + expect(res.body.message).toBeDefined() }) }) @@ -102,6 +105,7 @@ describe("no user role update in business", () => { }) expect(res.status).toBe(200) expect(res.body.data.roles["app_a"]).toBe("BASIC") + expect(res.body.message).toBeUndefined() }) it("should allow 'admin' to be updated", async () => { @@ -112,6 +116,7 @@ describe("no user role update in business", () => { }) expect(res.status).toBe(200) expect(res.body.data.admin.global).toBe(true) + expect(res.body.message).toBeUndefined() }) it("should allow 'builder' to be updated", async () => { @@ -122,5 +127,6 @@ describe("no user role update in business", () => { }) expect(res.status).toBe(200) expect(res.body.data.builder.global).toBe(true) + expect(res.body.message).toBeUndefined() }) }) diff --git a/packages/server/src/utilities/users.ts b/packages/server/src/utilities/users.ts index f841ec3646..bbc1370355 100644 --- a/packages/server/src/utilities/users.ts +++ b/packages/server/src/utilities/users.ts @@ -36,5 +36,8 @@ export function publicApiUserFix(ctx: UserCtx) { if (!ctx.request.body._id && ctx.params.userId) { ctx.request.body._id = ctx.params.userId } + if (!ctx.request.body.roles) { + ctx.request.body.roles = {} + } return ctx }