diff --git a/packages/server/specs/openapi.json b/packages/server/specs/openapi.json index 1e5718c5b5..5301d340bc 100644 --- a/packages/server/specs/openapi.json +++ b/packages/server/specs/openapi.json @@ -1550,8 +1550,7 @@ } }, "required": [ - "email", - "roles" + "email" ] }, "userOutput": { @@ -1622,7 +1621,6 @@ }, "required": [ "email", - "roles", "_id" ] } @@ -1701,7 +1699,6 @@ }, "required": [ "email", - "roles", "_id" ] } diff --git a/packages/server/specs/openapi.yaml b/packages/server/specs/openapi.yaml index 07320917b8..a525ec3be1 100644 --- a/packages/server/specs/openapi.yaml +++ b/packages/server/specs/openapi.yaml @@ -1324,7 +1324,6 @@ components: role ID, e.g. ADMIN. required: - email - - roles userOutput: type: object properties: @@ -1385,7 +1384,6 @@ components: type: string required: - email - - roles - _id required: - data @@ -1451,7 +1449,6 @@ components: type: string required: - email - - roles - _id required: - data diff --git a/packages/server/specs/resources/user.ts b/packages/server/specs/resources/user.ts index d00ed02f81..caf736f1ab 100644 --- a/packages/server/specs/resources/user.ts +++ b/packages/server/specs/resources/user.ts @@ -92,7 +92,7 @@ const userSchema = object( }, }, }, - { required: ["email", "roles"] } + { required: ["email"] } ) const userOutputSchema = { 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/definitions/openapi.ts b/packages/server/src/definitions/openapi.ts index fe5c17b218..5c44b6259f 100644 --- a/packages/server/src/definitions/openapi.ts +++ b/packages/server/src/definitions/openapi.ts @@ -599,7 +599,7 @@ export interface components { global?: boolean; }; /** @description Contains the roles of the user per app (assuming they are not a builder user). This field can only be set on a business or enterprise license. */ - roles: { [key: string]: string }; + roles?: { [key: string]: string }; }; userOutput: { data: { @@ -629,7 +629,7 @@ export interface components { global?: boolean; }; /** @description Contains the roles of the user per app (assuming they are not a builder user). This field can only be set on a business or enterprise license. */ - roles: { [key: string]: string }; + roles?: { [key: string]: string }; /** @description The ID of the user. */ _id: string; }; @@ -662,7 +662,7 @@ export interface components { global?: boolean; }; /** @description Contains the roles of the user per app (assuming they are not a builder user). This field can only be set on a business or enterprise license. */ - roles: { [key: string]: string }; + roles?: { [key: string]: string }; /** @description The ID of the user. */ _id: string; }[]; 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 } diff --git a/specs/openapi.json b/specs/openapi.json deleted file mode 120000 index 6e1531e3f4..0000000000 --- a/specs/openapi.json +++ /dev/null @@ -1 +0,0 @@ -../packages/server/specs/openapi.json \ No newline at end of file diff --git a/specs/openapi.yaml b/specs/openapi.yaml deleted file mode 120000 index 67db6af71f..0000000000 --- a/specs/openapi.yaml +++ /dev/null @@ -1 +0,0 @@ -../packages/server/specs/openapi.yaml \ No newline at end of file