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
This commit is contained in:
melohagan 2024-08-30 17:29:38 +01:00 committed by GitHub
parent ab0d658930
commit 3f357561d0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 119 additions and 44 deletions

View File

@ -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
}
}

View File

@ -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}

View File

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

View File

@ -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>

View File

@ -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"

View File

@ -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">

View File

@ -198,7 +198,7 @@ export const createLicensingStore = () => {
}, {})
}
const monthlyMetrics = getMetrics(
["dayPasses", "queries", "automations"],
["queries", "automations"],
license.quotas.usage.monthly,
usage.monthly.current
)

View File

@ -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,
}
}

View File

@ -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}`,
})
},
})

View File

@ -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",

View File

@ -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 = {

View File

@ -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

View File

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