From 663c0f20af3b4057d18db996020f7efd37e53eb5 Mon Sep 17 00:00:00 2001
From: Andrew Kingston <andrew@kingston.dev>
Date: Fri, 13 Sep 2024 14:14:36 +0100
Subject: [PATCH] Don't use display names as role names, and restore usual
 names for built in roles

---
 packages/backend-core/src/security/roles.ts   | 84 +++++++++++++------
 packages/server/src/api/controllers/role.ts   |  2 +-
 .../src/api/routes/global/tests/roles.spec.ts |  1 +
 3 files changed, 59 insertions(+), 28 deletions(-)

diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts
index a7210ec2b8..65339832cf 100644
--- a/packages/backend-core/src/security/roles.ts
+++ b/packages/backend-core/src/security/roles.ts
@@ -48,9 +48,14 @@ export class Role implements RoleDoc {
   permissions: Record<string, PermissionLevel[]> = {}
   uiMetadata?: RoleUIMetadata
 
-  constructor(id: string, permissionId: string, uiMetadata?: RoleUIMetadata) {
+  constructor(
+    id: string,
+    name: string,
+    permissionId: string,
+    uiMetadata?: RoleUIMetadata
+  ) {
     this._id = id
-    this.name = uiMetadata?.displayName || id
+    this.name = name
     this.uiMetadata = uiMetadata
     this.permissionId = permissionId
     // version for managing the ID - removing the role_ when responding
@@ -64,31 +69,56 @@ export class Role implements RoleDoc {
 }
 
 const BUILTIN_ROLES = {
-  ADMIN: new Role(BUILTIN_IDS.ADMIN, BuiltinPermissionID.ADMIN, {
-    displayName: "App admin",
-    description: "Can do everything",
-    color: RoleColor.ADMIN,
-  }).addInheritance(BUILTIN_IDS.POWER),
-  POWER: new Role(BUILTIN_IDS.POWER, BuiltinPermissionID.POWER, {
-    displayName: "App power user",
-    description: "An app user with more access",
-    color: RoleColor.POWER,
-  }).addInheritance(BUILTIN_IDS.BASIC),
-  BASIC: new Role(BUILTIN_IDS.BASIC, BuiltinPermissionID.WRITE, {
-    displayName: "App user",
-    description: "Any logged in user",
-    color: RoleColor.BASIC,
-  }).addInheritance(BUILTIN_IDS.PUBLIC),
-  PUBLIC: new Role(BUILTIN_IDS.PUBLIC, BuiltinPermissionID.PUBLIC, {
-    displayName: "Public user",
-    description: "Accessible to anyone",
-    color: RoleColor.PUBLIC,
-  }),
-  BUILDER: new Role(BUILTIN_IDS.BUILDER, BuiltinPermissionID.ADMIN, {
-    displayName: "Builder user",
-    description: "Users that can edit this app",
-    color: RoleColor.BUILDER,
-  }),
+  ADMIN: new Role(
+    BUILTIN_IDS.ADMIN,
+    BUILTIN_IDS.ADMIN,
+    BuiltinPermissionID.ADMIN,
+    {
+      displayName: "App admin",
+      description: "Can do everything",
+      color: RoleColor.ADMIN,
+    }
+  ).addInheritance(BUILTIN_IDS.POWER),
+  POWER: new Role(
+    BUILTIN_IDS.POWER,
+    BUILTIN_IDS.POWER,
+    BuiltinPermissionID.POWER,
+    {
+      displayName: "App power user",
+      description: "An app user with more access",
+      color: RoleColor.POWER,
+    }
+  ).addInheritance(BUILTIN_IDS.BASIC),
+  BASIC: new Role(
+    BUILTIN_IDS.BASIC,
+    BUILTIN_IDS.BASIC,
+    BuiltinPermissionID.WRITE,
+    {
+      displayName: "App user",
+      description: "Any logged in user",
+      color: RoleColor.BASIC,
+    }
+  ).addInheritance(BUILTIN_IDS.PUBLIC),
+  PUBLIC: new Role(
+    BUILTIN_IDS.PUBLIC,
+    BUILTIN_IDS.PUBLIC,
+    BuiltinPermissionID.PUBLIC,
+    {
+      displayName: "Public user",
+      description: "Accessible to anyone",
+      color: RoleColor.PUBLIC,
+    }
+  ),
+  BUILDER: new Role(
+    BUILTIN_IDS.BUILDER,
+    BUILTIN_IDS.BUILDER,
+    BuiltinPermissionID.ADMIN,
+    {
+      displayName: "Builder user",
+      description: "Users that can edit this app",
+      color: RoleColor.BUILDER,
+    }
+  ),
 }
 
 export function getBuiltinRoles(): { [key: string]: RoleDoc } {
diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts
index ee1c223952..b6b9ac1a29 100644
--- a/packages/server/src/api/controllers/role.ts
+++ b/packages/server/src/api/controllers/role.ts
@@ -89,7 +89,7 @@ export async function save(ctx: UserCtx<SaveRoleRequest, SaveRoleResponse>) {
     ctx.throw(400, "Cannot change custom role name")
   }
 
-  const role = new roles.Role(_id, permissionId, {
+  const role = new roles.Role(_id, name, permissionId, {
     displayName: uiMetadata?.displayName || name,
     description: uiMetadata?.description || "Custom role",
     color: uiMetadata?.color || RoleColor.DEFAULT_CUSTOM,
diff --git a/packages/worker/src/api/routes/global/tests/roles.spec.ts b/packages/worker/src/api/routes/global/tests/roles.spec.ts
index 35060d65fb..11de06328e 100644
--- a/packages/worker/src/api/routes/global/tests/roles.spec.ts
+++ b/packages/worker/src/api/routes/global/tests/roles.spec.ts
@@ -35,6 +35,7 @@ describe("/api/global/roles", () => {
 
   const role = new roles.Role(
     db.generateRoleID(ROLE_NAME),
+    ROLE_NAME,
     permissions.BuiltinPermissionID.READ_ONLY,
     { displayName: roles.BUILTIN_ROLE_IDS.BASIC }
   )