diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 883a29df64..ff8dd3609c 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -55,11 +55,19 @@ export async function save(ctx: UserCtx) { const db = context.getAppDB() let { _id, name, inherits, permissionId, version } = ctx.request.body let isCreate = false - if (!_id || version === roles.RoleVersion.VERSION_2) { + + if (_id && roles.isBuiltin(_id)) { + ctx.throw(400, "Cannot update builtin roles.") + } + + // if not id found, then its creation + if (!_id) { _id = generateRoleID(name) isCreate = true - } else if (roles.isBuiltin(_id)) { - ctx.throw(400, "Cannot update builtin roles.") + } + // version 2 roles need updated to add back role_ + else if (version === roles.RoleVersion.VERSION_2) { + _id = generateRoleID(name) } const role = new roles.Role(_id, name, permissionId).addInheritance(inherits) diff --git a/packages/server/src/api/routes/tests/role.spec.js b/packages/server/src/api/routes/tests/role.spec.js index f42ffe4846..5282597897 100644 --- a/packages/server/src/api/routes/tests/role.spec.js +++ b/packages/server/src/api/routes/tests/role.spec.js @@ -29,10 +29,11 @@ describe("/roles", () => { describe("create", () => { it("returns a success message when role is successfully created", async () => { - const res = await createRole() + const role = basicRole() + const res = await createRole(role) expect(res.res.statusMessage).toEqual( - "Role 'NewRole' created successfully." + `Role '${role.name}' created successfully.` ) expect(res.body._id).toBeDefined() expect(res.body._rev).toBeDefined() @@ -44,12 +45,13 @@ describe("/roles", () => { describe("update", () => { it("updates a role", async () => { - let res = await createRole() + const role = basicRole() + let res = await createRole(role) jest.clearAllMocks() res = await createRole(res.body) expect(res.res.statusMessage).toEqual( - "Role 'NewRole' created successfully." + `Role '${role.name}' created successfully.` ) expect(res.body._id).toBeDefined() expect(res.body._rev).toBeDefined() @@ -86,7 +88,7 @@ describe("/roles", () => { expect(powerUserRole.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC) expect(powerUserRole.permissionId).toEqual(BuiltinPermissionID.POWER) - const customRoleFetched = res.body.find(r => r._id === customRole._id) + const customRoleFetched = res.body.find(r => r._id === customRole.name) expect(customRoleFetched).toBeDefined() expect(customRoleFetched.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC) expect(customRoleFetched.permissionId).toEqual( diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts index 31b052336e..fc8a2b3e98 100644 --- a/packages/server/src/tests/utilities/structures.ts +++ b/packages/server/src/tests/utilities/structures.ts @@ -266,6 +266,7 @@ export function basicRole() { name: `NewRole_${utils.newid()}`, inherits: roles.BUILTIN_ROLE_IDS.BASIC, permissionId: permissions.BuiltinPermissionID.READ_ONLY, + version: 2, } }