From 3f357561d0f42755845381d15d02e8e84058ba9a Mon Sep 17 00:00:00 2001
From: melohagan <101575380+melohagan@users.noreply.github.com>
Date: Fri, 30 Aug 2024 17:29:38 +0100
Subject: [PATCH] The UI should indicate who the account holder is (#14470)

* Get tenantInfo in user fetch

* Add account holder label in users table

* Don't allow account holder to be selected in users table

* Sort account holder to top of list

* Only use account holder role in users table

* lint

* Remove joi validation from tenant-info endpoint

* Remove dayPasses

* Catch CouchDB 404 and return undefined

* Don't allow account holder role to be changed UI

* Don't offer delete option for tenant owner

* Backend validation to ensure account holder role cannot be updated

* Don't allow account holder role to be changed UI

* Get tenantOwner in separate call

* Pass data into SelectEditRenderer

* Rename var to __selectable

* setEnrichedUsers

* Update pro reference

* Only load tenantOwner once
---
 packages/backend-core/src/tenancy/db.ts       | 14 +++++++---
 .../bbui/src/Table/SelectEditRenderer.svelte  |  3 ++-
 packages/bbui/src/Table/Table.svelte          | 12 ++++++++-
 .../portal/users/users/[userId].svelte        | 20 +++++++++-----
 .../_components/RoleTableRenderer.svelte      |  2 +-
 .../builder/portal/users/users/index.svelte   | 23 +++++++++++++---
 .../builder/src/stores/portal/licensing.js    |  2 +-
 packages/builder/src/stores/portal/users.js   | 10 ++++++-
 packages/frontend-core/src/api/user.js        |  6 +++++
 packages/frontend-core/src/constants.js       | 11 ++++++++
 .../src/api/controllers/global/users.ts       | 11 ++++++++
 .../worker/src/api/routes/global/tenant.ts    | 27 +------------------
 .../src/api/routes/global/tests/users.spec.ts | 22 +++++++++++++++
 13 files changed, 119 insertions(+), 44 deletions(-)

diff --git a/packages/backend-core/src/tenancy/db.ts b/packages/backend-core/src/tenancy/db.ts
index d430296a5c..332ecbca48 100644
--- a/packages/backend-core/src/tenancy/db.ts
+++ b/packages/backend-core/src/tenancy/db.ts
@@ -15,7 +15,15 @@ export async function saveTenantInfo(tenantInfo: TenantInfo) {
   })
 }
 
-export async function getTenantInfo(tenantId: string): Promise<TenantInfo> {
-  const db = getTenantDB(tenantId)
-  return db.get("tenant_info")
+export async function getTenantInfo(
+  tenantId: string
+): Promise<TenantInfo | undefined> {
+  try {
+    const db = getTenantDB(tenantId)
+    const tenantInfo = (await db.get("tenant_info")) as TenantInfo
+    delete tenantInfo.owner.password
+    return tenantInfo
+  } catch {
+    return undefined
+  }
 }
diff --git a/packages/bbui/src/Table/SelectEditRenderer.svelte b/packages/bbui/src/Table/SelectEditRenderer.svelte
index c6eafa3ed0..0c88fa3167 100644
--- a/packages/bbui/src/Table/SelectEditRenderer.svelte
+++ b/packages/bbui/src/Table/SelectEditRenderer.svelte
@@ -6,10 +6,11 @@
   export let onEdit
   export let allowSelectRows = false
   export let allowEditRows = false
+  export let data
 </script>
 
 <div>
-  {#if allowSelectRows}
+  {#if allowSelectRows && data.__selectable !== false}
     <Checkbox value={selected} />
   {/if}
   {#if allowEditRows}
diff --git a/packages/bbui/src/Table/Table.svelte b/packages/bbui/src/Table/Table.svelte
index 868f7b3a0b..3916bec738 100644
--- a/packages/bbui/src/Table/Table.svelte
+++ b/packages/bbui/src/Table/Table.svelte
@@ -43,6 +43,8 @@
   export let showHeaderBorder = true
   export let placeholderText = "No rows found"
   export let snippets = []
+  export let defaultSortColumn
+  export let defaultSortOrder = "Ascending"
 
   const dispatch = createEventDispatcher()
 
@@ -162,6 +164,8 @@
   }
 
   const sortRows = (rows, sortColumn, sortOrder) => {
+    sortColumn = sortColumn ?? defaultSortColumn
+    sortOrder = sortOrder ?? defaultSortOrder
     if (!sortColumn || !sortOrder || disableSorting) {
       return rows
     }
@@ -259,7 +263,10 @@
     if (select) {
       // Add any rows which are not already in selected rows
       rows.forEach(row => {
-        if (selectedRows.findIndex(x => x._id === row._id) === -1) {
+        if (
+          row.__selectable !== false &&
+          selectedRows.findIndex(x => x._id === row._id) === -1
+        ) {
           selectedRows.push(row)
         }
       })
@@ -396,6 +403,9 @@
                   class:noBorderCheckbox={!showHeaderBorder}
                   class="spectrum-Table-cell spectrum-Table-cell--divider spectrum-Table-cell--edit"
                   on:click={e => {
+                    if (row.__selectable === false) {
+                      return
+                    }
                     toggleSelectRow(row)
                     e.stopPropagation()
                   }}
diff --git a/packages/builder/src/pages/builder/portal/users/users/[userId].svelte b/packages/builder/src/pages/builder/portal/users/users/[userId].svelte
index d4e765a4be..458c9a3f79 100644
--- a/packages/builder/src/pages/builder/portal/users/users/[userId].svelte
+++ b/packages/builder/src/pages/builder/portal/users/users/[userId].svelte
@@ -85,7 +85,7 @@
   let popoverAnchor
   let searchTerm = ""
   let popover
-  let user
+  let user, tenantOwner
   let loaded = false
 
   $: internalGroups = $groups?.filter(g => !g?.scimInfo?.isSync)
@@ -104,6 +104,7 @@
     })
   })
   $: globalRole = users.getUserRole(user)
+  $: isTenantOwner = tenantOwner?.email && tenantOwner.email === user?.email
 
   const getAvailableApps = (appList, privileged, roles) => {
     let availableApps = appList.slice()
@@ -205,6 +206,7 @@
     if (!user?._id) {
       $goto("./")
     }
+    tenantOwner = await users.tenantOwner($auth.tenantId)
   }
 
   async function toggleFlags(detail) {
@@ -268,9 +270,11 @@
                 Force password reset
               </MenuItem>
             {/if}
-            <MenuItem on:click={deleteModal.show} icon="Delete">
-              Delete
-            </MenuItem>
+            {#if !isTenantOwner}
+              <MenuItem on:click={deleteModal.show} icon="Delete">
+                Delete
+              </MenuItem>
+            {/if}
           </ActionMenu>
         </div>
       {/if}
@@ -310,9 +314,11 @@
             <Label size="L">Role</Label>
             <Select
               placeholder={null}
-              disabled={!sdk.users.isAdmin($auth.user)}
-              value={globalRole}
-              options={Constants.BudibaseRoleOptions}
+              disabled={!sdk.users.isAdmin($auth.user) || isTenantOwner}
+              value={isTenantOwner ? "owner" : globalRole}
+              options={isTenantOwner
+                ? Constants.ExtendedBudibaseRoleOptions
+                : Constants.BudibaseRoleOptions}
               on:change={updateUserRole}
             />
           </div>
diff --git a/packages/builder/src/pages/builder/portal/users/users/_components/RoleTableRenderer.svelte b/packages/builder/src/pages/builder/portal/users/users/_components/RoleTableRenderer.svelte
index 287e5a540f..0006cc4be0 100644
--- a/packages/builder/src/pages/builder/portal/users/users/_components/RoleTableRenderer.svelte
+++ b/packages/builder/src/pages/builder/portal/users/users/_components/RoleTableRenderer.svelte
@@ -4,7 +4,7 @@
 
   export let row
 
-  $: role = Constants.BudibaseRoleOptions.find(
+  $: role = Constants.ExtendedBudibaseRoleOptions.find(
     x => x.value === users.getUserRole(row)
   )
   $: value = role?.label || "Not available"
diff --git a/packages/builder/src/pages/builder/portal/users/users/index.svelte b/packages/builder/src/pages/builder/portal/users/users/index.svelte
index 01d23afa67..fab79db4a3 100644
--- a/packages/builder/src/pages/builder/portal/users/users/index.svelte
+++ b/packages/builder/src/pages/builder/portal/users/users/index.svelte
@@ -52,6 +52,7 @@
 
   let groupsLoaded = !$licensing.groupsEnabled || $groups?.length
   let enrichedUsers = []
+  let tenantOwner
   let createUserModal,
     inviteConfirmationModal,
     onboardingTypeModal,
@@ -70,6 +71,7 @@
   ]
   let userData = []
   let invitesLoaded = false
+  let tenantOwnerLoaded = false
   let pendingInvites = []
   let parsedInvites = []
 
@@ -98,8 +100,14 @@
   $: pendingSchema = getPendingSchema(schema)
   $: userData = []
   $: inviteUsersResponse = { successful: [], unsuccessful: [] }
-  $: {
-    enrichedUsers = $fetch.rows?.map(user => {
+  $: setEnrichedUsers($fetch.rows)
+
+  const setEnrichedUsers = async rows => {
+    if (!tenantOwnerLoaded) {
+      enrichedUsers = []
+      return
+    }
+    enrichedUsers = rows?.map(user => {
       let userGroups = []
       $groups.forEach(group => {
         if (group.users) {
@@ -110,15 +118,21 @@
           })
         }
       })
+      user.tenantOwnerEmail = tenantOwner?.email
+      const role = Constants.ExtendedBudibaseRoleOptions.find(
+        x => x.value === users.getUserRole(user)
+      )
       return {
         ...user,
         name: user.firstName ? user.firstName + " " + user.lastName : "",
         userGroups,
+        __selectable:
+          role.value === Constants.BudibaseRoles.Owner ? false : undefined,
         apps: [...new Set(Object.keys(user.roles))],
+        access: role.sortOrder,
       }
     })
   }
-
   const getPendingSchema = tblSchema => {
     if (!tblSchema) {
       return {}
@@ -302,6 +316,8 @@
       groupsLoaded = true
       pendingInvites = await users.getInvites()
       invitesLoaded = true
+      tenantOwner = await users.tenantOwner($auth.tenantId)
+      tenantOwnerLoaded = true
     } catch (error) {
       notifications.error("Error fetching user group data")
     }
@@ -376,6 +392,7 @@
     allowSelectRows={!readonly}
     {customRenderers}
     loading={!$fetch.loaded || !groupsLoaded}
+    defaultSortColumn={"access"}
   />
 
   <div class="pagination">
diff --git a/packages/builder/src/stores/portal/licensing.js b/packages/builder/src/stores/portal/licensing.js
index 41da5e0398..0e44650479 100644
--- a/packages/builder/src/stores/portal/licensing.js
+++ b/packages/builder/src/stores/portal/licensing.js
@@ -198,7 +198,7 @@ export const createLicensingStore = () => {
         }, {})
       }
       const monthlyMetrics = getMetrics(
-        ["dayPasses", "queries", "automations"],
+        ["queries", "automations"],
         license.quotas.usage.monthly,
         usage.monthly.current
       )
diff --git a/packages/builder/src/stores/portal/users.js b/packages/builder/src/stores/portal/users.js
index 80ddf0a737..e9e65c3803 100644
--- a/packages/builder/src/stores/portal/users.js
+++ b/packages/builder/src/stores/portal/users.js
@@ -128,8 +128,15 @@ export function createUsersStore() {
     return await API.removeAppBuilder({ userId, appId })
   }
 
+  async function getTenantOwner(tenantId) {
+    const tenantInfo = await API.getTenantInfo({ tenantId })
+    return tenantInfo?.owner
+  }
+
   const getUserRole = user => {
-    if (sdk.users.isAdmin(user)) {
+    if (user && user.email === user.tenantOwnerEmail) {
+      return Constants.BudibaseRoles.Owner
+    } else if (sdk.users.isAdmin(user)) {
       return Constants.BudibaseRoles.Admin
     } else if (sdk.users.isBuilder(user)) {
       return Constants.BudibaseRoles.Developer
@@ -169,6 +176,7 @@ export function createUsersStore() {
     save: refreshUsage(save),
     bulkDelete: refreshUsage(bulkDelete),
     delete: refreshUsage(del),
+    tenantOwner: getTenantOwner,
   }
 }
 
diff --git a/packages/frontend-core/src/api/user.js b/packages/frontend-core/src/api/user.js
index 6a4932d4af..a2846c1e7b 100644
--- a/packages/frontend-core/src/api/user.js
+++ b/packages/frontend-core/src/api/user.js
@@ -295,4 +295,10 @@ export const buildUserEndpoints = API => ({
       url: `/api/global/users/${userId}/app/${appId}/builder`,
     })
   },
+
+  getTenantInfo: async ({ tenantId }) => {
+    return await API.get({
+      url: `/api/global/tenant/${tenantId}`,
+    })
+  },
 })
diff --git a/packages/frontend-core/src/constants.js b/packages/frontend-core/src/constants.js
index 22e5e8583a..32d2d27e49 100644
--- a/packages/frontend-core/src/constants.js
+++ b/packages/frontend-core/src/constants.js
@@ -41,6 +41,7 @@ export const BudibaseRoles = {
   Developer: "developer",
   Creator: "creator",
   Admin: "admin",
+  Owner: "owner",
 }
 
 export const BudibaseRoleOptionsOld = [
@@ -54,18 +55,28 @@ export const BudibaseRoleOptions = [
     label: "Account admin",
     value: BudibaseRoles.Admin,
     subtitle: "Has full access to all apps and settings in your account",
+    sortOrder: 1,
   },
   {
     label: "Creator",
     value: BudibaseRoles.Creator,
     subtitle: "Can create and edit apps they have access to",
+    sortOrder: 2,
   },
   {
     label: "App user",
     value: BudibaseRoles.AppUser,
     subtitle: "Can only use published apps they have access to",
+    sortOrder: 3,
   },
 ]
+export const ExtendedBudibaseRoleOptions = [
+  {
+    label: "Account holder",
+    value: BudibaseRoles.Owner,
+    sortOrder: 0,
+  },
+].concat(BudibaseRoleOptions)
 
 export const PlanType = {
   FREE: "free",
diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts
index b039376d9b..6ce0eef5a0 100644
--- a/packages/worker/src/api/controllers/global/users.ts
+++ b/packages/worker/src/api/controllers/global/users.ts
@@ -54,6 +54,17 @@ export const save = async (ctx: UserCtx<User, SaveUserResponse>) => {
     const currentUserId = ctx.user?._id
     const requestUser = ctx.request.body
 
+    // Do not allow the account holder role to be changed
+    const tenantInfo = await tenancy.getTenantInfo(requestUser.tenantId)
+    if (tenantInfo?.owner.email === requestUser.email) {
+      if (
+        requestUser.admin?.global !== true ||
+        requestUser.builder?.global !== true
+      ) {
+        throw Error("Cannot set role of account holder")
+      }
+    }
+
     const user = await userSdk.db.save(requestUser, { currentUserId })
 
     ctx.body = {
diff --git a/packages/worker/src/api/routes/global/tenant.ts b/packages/worker/src/api/routes/global/tenant.ts
index 18b7058c34..f15e374774 100644
--- a/packages/worker/src/api/routes/global/tenant.ts
+++ b/packages/worker/src/api/routes/global/tenant.ts
@@ -1,36 +1,11 @@
 import Router from "@koa/router"
-import Joi from "joi"
-import { auth } from "@budibase/backend-core"
 import * as controller from "../../controllers/global/tenant"
 import cloudRestricted from "../../../middleware/cloudRestricted"
 
 const router: Router = new Router()
-const OPTIONAL_STRING = Joi.string().optional().allow(null).allow("")
-
-function buildTenantInfoValidation() {
-  return auth.joiValidator.body(
-    Joi.object({
-      owner: Joi.object({
-        email: Joi.string().required(),
-        password: OPTIONAL_STRING,
-        ssoId: OPTIONAL_STRING,
-        givenName: OPTIONAL_STRING,
-        familyName: OPTIONAL_STRING,
-        budibaseUserId: OPTIONAL_STRING,
-      }).required(),
-      hosting: Joi.string().required(),
-      tenantId: Joi.string().required(),
-    }).required()
-  )
-}
 
 router
-  .post(
-    "/api/global/tenant",
-    cloudRestricted,
-    buildTenantInfoValidation(),
-    controller.save
-  )
+  .post("/api/global/tenant", cloudRestricted, controller.save)
   .get("/api/global/tenant/:id", controller.get)
 
 export default router
diff --git a/packages/worker/src/api/routes/global/tests/users.spec.ts b/packages/worker/src/api/routes/global/tests/users.spec.ts
index fe6089a621..a654c42359 100644
--- a/packages/worker/src/api/routes/global/tests/users.spec.ts
+++ b/packages/worker/src/api/routes/global/tests/users.spec.ts
@@ -412,6 +412,28 @@ describe("/api/global/users", () => {
       expect(events.user.permissionBuilderRemoved).toHaveBeenCalledTimes(1)
     })
 
+    it("should not be able to update an account holder user to a basic user", async () => {
+      const accountHolderUser = await config.createUser(
+        structures.users.adminUser()
+      )
+      jest.clearAllMocks()
+      tenancy.getTenantInfo = jest.fn().mockImplementation(() => ({
+        owner: {
+          email: accountHolderUser.email,
+        },
+      }))
+
+      accountHolderUser.admin!.global = false
+      accountHolderUser.builder!.global = false
+
+      await config.api.users.saveUser(accountHolderUser, 400)
+
+      expect(events.user.created).not.toHaveBeenCalled()
+      expect(events.user.updated).not.toHaveBeenCalled()
+      expect(events.user.permissionAdminRemoved).not.toHaveBeenCalled()
+      expect(events.user.permissionBuilderRemoved).not.toHaveBeenCalled()
+    })
+
     it("should be able to update an builder user to a basic user", async () => {
       const user = await config.createUser(structures.users.builderUser())
       jest.clearAllMocks()