Merge pull request #11827 from Budibase/fix/role-required-spec

Small user openAPI update

packages/server/specs/openapi.json

@@ -1550,8 +1550,7 @@
           }
         },
         "required": [
-          "email",
+          "email"
-          "roles"
         ]
       },
       "userOutput": {
@@ -1622,7 +1621,6 @@
             },
             "required": [
               "email",
-              "roles",
               "_id"
             ]
           }
@@ -1701,7 +1699,6 @@
               },
               "required": [
                 "email",
-                "roles",
                 "_id"
               ]
             }

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

packages/server/specs/resources/user.ts

@@ -92,7 +92,7 @@ const userSchema = object(
       },
     },
   },
-  { required: ["email", "roles"] }
+  { required: ["email"] }
 )
 
 const userOutputSchema = {

packages/server/src/api/controllers/public/mapping/users.ts

@@ -15,10 +15,15 @@ function user(body: any): User {
   }
 }
 
-function mapUser(ctx: any): { data: User } {
+function mapUser(ctx: any) {
-  return {
+  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[] } {

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))
+  await createUpdateResponse(ctx)
-  const response = await saveGlobalUser(ctx)
-  ctx.body = await getUser(ctx, response._id)
   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))
+  await createUpdateResponse(ctx, user)
-  const response = await saveGlobalUser(ctx)
-  ctx.body = await getUser(ctx, response._id)
   await next()
 }
 

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()
   })
 })

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;
       }[];

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
 }

specs/openapi.json

@@ -1 +0,0 @@
-../packages/server/specs/openapi.json

specs/openapi.yaml

@@ -1 +0,0 @@
-../packages/server/specs/openapi.yaml