diff --git a/apps/nextjs/src/app/[locale]/manage/_access.ts b/apps/nextjs/src/app/[locale]/manage/_access.ts new file mode 100644 index 000000000..89d621e6e --- /dev/null +++ b/apps/nextjs/src/app/[locale]/manage/_access.ts @@ -0,0 +1,78 @@ +import type { Session } from "@homarr/auth"; + +import { api } from "@homarr/api/server"; + +/** + * Access rules for the management sections, shared by the navigation and the pages themselves so + * the two can never drift apart. The rule is always the same: a section is reachable when the user + * can do at least one thing in it, and the section only ever lists what the user is allowed to see. + */ + +export interface AppsSectionAccess { + /** May see and edit every app */ + canManageAll: boolean; + /** May add new apps */ + canCreate: boolean; + /** May delete apps */ + canDelete: boolean; + /** May open /manage/apps at all */ + canAccess: boolean; +} + +export const getAppsSectionAccess = (session: Session | null): AppsSectionAccess => { + const permissions = session?.user.permissions ?? []; + const canManageAll = permissions.includes("app-modify-all"); + const canCreate = permissions.includes("app-create"); + + return { + canManageAll, + canCreate, + canDelete: permissions.includes("app-full-all"), + canAccess: canManageAll || canCreate, + }; +}; + +export interface IntegrationsSectionAccess { + /** May manage every integration */ + canManageAll: boolean; + /** May add new integrations */ + canCreate: boolean; + /** May manage at least one specific integration through a delegated grant */ + hasDelegatedAccess: boolean; + /** May open /manage/integrations at all */ + canAccess: boolean; +} + +/** + * Pure variant for callers that already fetched the integrations the user can see, so the page and + * the navigation never disagree and the list is only queried once per request. + */ +export const getIntegrationsSectionAccess = ( + session: Session | null, + visibleIntegrations: { permissions: { hasFullAccess: boolean } }[], +): IntegrationsSectionAccess => { + const permissions = session?.user.permissions ?? []; + const canManageAll = permissions.includes("integration-full-all"); + const canCreate = permissions.includes("integration-create"); + const hasDelegatedAccess = + !canManageAll && visibleIntegrations.some((integration) => integration.permissions.hasFullAccess); + + return { + canManageAll, + canCreate, + hasDelegatedAccess, + canAccess: canManageAll || canCreate || hasDelegatedAccess, + }; +}; + +export const getIntegrationsSectionAccessAsync = async ( + session: Session | null, +): Promise => { + const permissions = session?.user.permissions ?? []; + + // Only pay for the lookup when the answer is not already known from the global permissions. + const needsDelegatedLookup = + session !== null && !permissions.includes("integration-full-all") && !permissions.includes("integration-create"); + + return getIntegrationsSectionAccess(session, needsDelegatedLookup ? await api.integration.all() : []); +}; diff --git a/apps/nextjs/src/app/[locale]/manage/apps/page.tsx b/apps/nextjs/src/app/[locale]/manage/apps/page.tsx index 536bd6ca8..7f6211b00 100644 --- a/apps/nextjs/src/app/[locale]/manage/apps/page.tsx +++ b/apps/nextjs/src/app/[locale]/manage/apps/page.tsx @@ -16,6 +16,7 @@ import { TourTarget } from "~/components/layout/header/tour-target"; import { ManageCollectionItem, ManageCollectionPage } from "~/components/manage/manage-collection"; import { MobileAffixButton } from "~/components/manage/mobile-affix-button"; import { NoResults } from "~/components/no-results"; +import { getAppsSectionAccess } from "../_access"; import { AppDeleteButton } from "./_app-delete-button"; const searchParamsSchema = z.object({ @@ -41,16 +42,26 @@ interface AppsPageProps { export default async function AppsPage(props: AppsPageProps) { const session = await auth(); if (!session) redirect("/auth/login"); - if (!session.user.permissions.includes("app-modify-all")) notFound(); + const { canManageAll, canCreate, canDelete, canAccess } = getAppsSectionAccess(session); + if (!canAccess) notFound(); const searchParams = searchParamsSchema.parse(await props.searchParams); - const { items: apps, totalCount } = await api.app.getPaginated(searchParams); + // Without app-modify-all the user may add apps but not browse the ones they cannot manage, + // so the list stays empty instead of leaking every app name and URL. + const { items: apps, totalCount } = canManageAll + ? await api.app.getPaginated(searchParams) + : { items: [], totalCount: 0 }; const t = await getScopedI18n("app"); - const canCreate = session.user.permissions.includes("app-create"); - const canDelete = session.user.permissions.includes("app-full-all"); - const hasSearch = Boolean(searchParams.search?.trim()); + const hasSearch = canManageAll && Boolean(searchParams.search?.trim()); - const emptyState = hasSearch ? ( + const emptyState = !canManageAll ? ( + + ) : hasSearch ? ( + canManageAll ? ( + + ) : undefined } footer={ totalCount > searchParams.pageSize ? ( diff --git a/apps/nextjs/src/app/[locale]/manage/integrations/page.tsx b/apps/nextjs/src/app/[locale]/manage/integrations/page.tsx index 6488b5f26..df4c19ac9 100644 --- a/apps/nextjs/src/app/[locale]/manage/integrations/page.tsx +++ b/apps/nextjs/src/app/[locale]/manage/integrations/page.tsx @@ -1,4 +1,4 @@ -import { redirect } from "next/navigation"; +import { notFound, redirect } from "next/navigation"; import { ActionIcon, ActionIconGroup, Anchor, Badge, Stack, Text } from "@mantine/core"; import { IconPencil, IconPlugX } from "@tabler/icons-react"; import { z } from "zod/v4"; @@ -15,6 +15,7 @@ import { TourTarget } from "~/components/layout/header/tour-target"; import { ManageCollectionItem, ManageCollectionPage } from "~/components/manage/manage-collection"; import { MobileAffixButton } from "~/components/manage/mobile-affix-button"; import { NoResults } from "~/components/no-results"; +import { getIntegrationsSectionAccess } from "../_access"; import { DeleteIntegrationActionButton } from "./_integration-buttons"; const searchParamsSchema = z.object({ search: z.string().optional() }); @@ -29,14 +30,22 @@ export default async function IntegrationsPage(props: IntegrationsPageProps) { const searchParams = searchParamsSchema.parse(await props.searchParams); const integrations = await api.integration.all(); + const { + canManageAll: hasGlobalFullAccess, + canCreate, + canAccess, + } = getIntegrationsSectionAccess(session, integrations); + if (!canAccess) notFound(); + const t = await getScopedI18n("integration"); - const canCreate = session.user.permissions.includes("integration-create"); - const hasGlobalFullAccess = session.user.permissions.includes("integration-full-all"); - const canSee = hasGlobalFullAccess || integrations.some((integration) => integration.permissions.hasFullAccess); + // Without integration-full-all only the integrations that were explicitly delegated to the user + // are manageable, so the list never shows an integration they cannot open. + const manageableIntegrations = integrations.filter( + (integration) => hasGlobalFullAccess || integration.permissions.hasFullAccess, + ); const query = searchParams.search?.trim().toLocaleLowerCase() ?? ""; - const filteredIntegrations = integrations - .filter((integration) => hasGlobalFullAccess || integration.permissions.hasFullAccess) + const filteredIntegrations = manageableIntegrations .filter((integration) => { if (!query) return true; return [integration.name, getIntegrationName(integration.kind), integration.url].some((value) => @@ -56,11 +65,12 @@ export default async function IntegrationsPage(props: IntegrationsPageProps) { ariaLabel={t("page.list.ariaLabel")} itemCount={filteredIntegrations.length} emptyState={ - !canSee ? ( + manageableIntegrations.length === 0 && !hasGlobalFullAccess ? ( ) : hasSearch ? ( integration.permissions.hasFullAccess) - : false; - const canManageIntegrations = hasGlobalIntegrationFullAccess || hasDelegatedIntegrationFullAccess; + const appsAccess = getAppsSectionAccess(session); + const integrationsAccess = await getIntegrationsSectionAccessAsync(session); const navigationLinks: NavigationLink[] = [ { label: t("items.home"), @@ -89,7 +85,7 @@ export default async function ManageLayout({ children }: PropsWithChildren) { icon: IconBox, href: "/manage/apps", label: t("items.apps"), - hidden: !session?.user.permissions.includes("app-modify-all"), + hidden: !appsAccess.canAccess, iconProps: { strokeWidth: 2.5, }, @@ -99,7 +95,7 @@ export default async function ManageLayout({ children }: PropsWithChildren) { icon: IconAffiliateFilled, href: "/manage/integrations", label: t("items.integrations"), - hidden: !canManageIntegrations, + hidden: !integrationsAccess.canAccess, "data-onboarding-tour-id": "manage-integrations", }, { diff --git a/apps/nextjs/src/app/[locale]/manage/users/groups/[id]/permissions/_group-permission-form.tsx b/apps/nextjs/src/app/[locale]/manage/users/groups/[id]/permissions/_group-permission-form.tsx index e91ec1d02..6908b62ba 100644 --- a/apps/nextjs/src/app/[locale]/manage/users/groups/[id]/permissions/_group-permission-form.tsx +++ b/apps/nextjs/src/app/[locale]/manage/users/groups/[id]/permissions/_group-permission-form.tsx @@ -1,16 +1,19 @@ "use client"; import type { PropsWithChildren } from "react"; -import { useCallback } from "react"; +import { useCallback, useId } from "react"; import { Badge, Button, Card, + Checkbox, Collapse, + Divider, Group, SegmentedControl, Stack, Text, + Tooltip, Transition, UnstyledButton, } from "@mantine/core"; @@ -23,6 +26,7 @@ import type { GroupPermissionKey, PermissionMatrixCategory, PermissionMatrixStat import { getPermissionsWithChildren, groupPermissionKeys, + isCreateImpliedByLevel, matrixStateToPermissions, permissionMatrix, permissionsToMatrixState, @@ -72,34 +76,57 @@ type MatrixForm = ReturnType; const emptyMatrixState = (): PermissionMatrixState => objectKeys(permissionMatrix).reduce((acc, category) => { - acc[category] = { level: 0 }; + acc[category] = { level: 0, create: false }; return acc; }, {} as PermissionMatrixState); -export const getMatrixLevel = (values: FormType, category: PermissionMatrixCategory): number => { - const checkedKeys = groupPermissionKeys.filter((key) => values[key]); - return permissionsToMatrixState(checkedKeys)[category].level; -}; +const matrixStateOf = (values: FormType) => permissionsToMatrixState(groupPermissionKeys.filter((key) => values[key])); + +export const getMatrixLevel = (values: FormType, category: PermissionMatrixCategory): number => + matrixStateOf(values)[category].level; + +export const hasMatrixCreate = (values: FormType, category: PermissionMatrixCategory): boolean => + matrixStateOf(values)[category].create; export const applyMatrixLevel = (form: MatrixForm, category: PermissionMatrixCategory, level: number) => { - const targetKeys = new Set(matrixStateToPermissions({ ...emptyMatrixState(), [category]: { level } })); + const targetKeys = new Set(matrixStateToPermissions({ ...emptyMatrixState(), [category]: { level, create: false } })); permissionMatrix[category].levels.forEach((key) => { form.setFieldValue(key, targetKeys.has(key)); }); + // The create key is deliberately left untouched: a level that implies creating already covers it, + // and an explicitly granted create must survive lowering the level again. +}; + +export const applyMatrixCreate = (form: MatrixForm, category: PermissionMatrixCategory, create: boolean) => { + const createKey = permissionMatrix[category].create; + if (createKey !== null) { + form.setFieldValue(createKey, create); + } }; const presets = { - viewer: { board: 1, app: 1, integration: 1, media: 1 }, - editor: { board: 2, app: 2, integration: 2, "search-engine": 1, media: 1 }, - admin: { admin: 1 }, -} satisfies Record>>; + viewer: { + board: { level: 1, create: false }, + app: { level: 1, create: false }, + integration: { level: 1, create: false }, + media: { level: 1, create: false }, + }, + editor: { + board: { level: 2, create: true }, + app: { level: 2, create: true }, + integration: { level: 2, create: true }, + "search-engine": { level: 1, create: true }, + media: { level: 1, create: true }, + }, + admin: { admin: { level: 1, create: false } }, +} satisfies Record>; export type PermissionPreset = keyof typeof presets; const presetToFormState = (preset: PermissionPreset): FormType => { - const levels: Partial> = presets[preset]; + const presetState: Partial = presets[preset]; const state = objectKeys(permissionMatrix).reduce((acc, category) => { - acc[category] = { level: levels[category] ?? 0 }; + acc[category] = presetState[category] ?? { level: 0, create: false }; return acc; }, {} as PermissionMatrixState); const enabledKeys = new Set(matrixStateToPermissions(state)); @@ -123,6 +150,19 @@ export const getActivePreset = (values: FormType): PermissionPreset | null => { ); }; +/** + * Short, scannable label for a level. Categories with a single level use their permission label + * instead, so the control reads "No access | View logs" rather than "No access | Enabled". + */ +const levelLabelKey = (key: GroupPermissionKey) => { + if (key.endsWith("-view-all")) return "view" as const; + if (key.endsWith("-use-all")) return "use" as const; + if (key.endsWith("-interact-all")) return "interact" as const; + if (key.endsWith("-modify-all")) return "modify" as const; + if (key.endsWith("-full-all")) return "full" as const; + return null; +}; + interface MatrixImpliedHintProps { category: PermissionMatrixCategory; permissionLabels: PermissionLabels; @@ -155,9 +195,10 @@ export const MatrixImpliedHint = ({ category, permissionLabels }: MatrixImpliedH interface MatrixRowProps { category: PermissionMatrixCategory; permissionLabels: PermissionLabels; + permissionDescriptions: PermissionLabels; } -export const MatrixRow = ({ category, permissionLabels }: MatrixRowProps) => { +export const MatrixRow = ({ category, permissionLabels, permissionDescriptions }: MatrixRowProps) => { const form = useFormContext(); const tPermissions = useScopedI18n("group.permission"); @@ -165,61 +206,116 @@ export const MatrixRow = ({ category, permissionLabels }: MatrixRowProps) => { const level = getMatrixLevel(values, category); const isDanger = category === "admin"; const disabledByAdmin = !isDanger && getMatrixLevel(values, "admin") > 0; + const { levels, create: createKey } = permissionMatrix[category]; + const hasSingleLevel = levels.length === 1; const data = [ { value: "0", label: tPermissions("matrix.none") }, - ...permissionMatrix[category].levels.map((key, index) => ({ - value: String(index + 1), - label: permissionLabels[key] ?? key, - })), + ...levels.map((key, index) => { + const shortKey = hasSingleLevel ? null : levelLabelKey(key); + return { + value: String(index + 1), + label: shortKey === null ? (permissionLabels[key] ?? key) : tPermissions(`matrix.level.${shortKey}`), + }; + }), ]; + const selectedKey = level === 0 ? undefined : levels[level - 1]; + const description = + selectedKey === undefined ? tPermissions("matrix.noneDescription") : (permissionDescriptions[selectedKey] ?? ""); + + const createImplied = isCreateImpliedByLevel(category, level); + const createChecked = hasMatrixCreate(values, category); + return ( - - + + {isDanger && } {tPermissions(`${category}.title`)} - applyMatrixLevel(form, category, Number(value))} - /> - {disabledByAdmin && ( - - {tPermissions("matrix.grantedViaAdmin")} - - )} + + applyMatrixLevel(form, category, Number(value))} + /> + {createKey !== null && ( + + applyMatrixCreate(form, category, event.currentTarget.checked)} + /> + + )} + + + {description} + + {disabledByAdmin && ( + + {tPermissions("matrix.grantedViaAdmin")} + + )} ); }; +interface PermissionMatrixProps { + permissionLabels: PermissionLabels; + permissionDescriptions: PermissionLabels; +} + +export const PermissionMatrix = ({ permissionLabels, permissionDescriptions }: PermissionMatrixProps) => ( + + + {objectKeys(permissionMatrix).map((category, index) => ( + + {index > 0 && } + + + ))} + + +); + export const PresetButtons = () => { const form = useFormContext(); const tPermissions = useScopedI18n("group.permission"); const activePreset = getActivePreset(form.getValues()); return ( - + {tPermissions("matrix.preset.label")} {(["viewer", "editor", "admin"] as const).map((preset) => ( - + + + ))} {activePreset === null && {tPermissions("matrix.preset.custom")}} @@ -234,6 +330,7 @@ export const EffectivePermissionPreview = ({ permissionLabels }: EffectivePermis const form = useFormContext(); const tPermissions = useScopedI18n("group.permission"); const [opened, { toggle }] = useDisclosure(false); + const panelId = useId(); const values = form.getValues(); const checkedKeys = groupPermissionKeys.filter((key) => values[key]); @@ -248,7 +345,7 @@ export const EffectivePermissionPreview = ({ permissionLabels }: EffectivePermis return ( - + {tPermissions("matrix.effective.title")} @@ -259,7 +356,7 @@ export const EffectivePermissionPreview = ({ permissionLabels }: EffectivePermis {effective.length} - + {grouped.length === 0 ? ( @@ -267,7 +364,7 @@ export const EffectivePermissionPreview = ({ permissionLabels }: EffectivePermis ) : ( grouped.map(({ category, keys }) => ( - + {tPermissions(`${category}.title`)} diff --git a/apps/nextjs/src/app/[locale]/manage/users/groups/[id]/permissions/page.tsx b/apps/nextjs/src/app/[locale]/manage/users/groups/[id]/permissions/page.tsx index 39e1b10d4..9b7900507 100644 --- a/apps/nextjs/src/app/[locale]/manage/users/groups/[id]/permissions/page.tsx +++ b/apps/nextjs/src/app/[locale]/manage/users/groups/[id]/permissions/page.tsx @@ -5,14 +5,14 @@ import { api } from "@homarr/api/server"; import { auth } from "@homarr/auth/next"; import { objectKeys } from "@homarr/common"; import type { GroupPermissionKey } from "@homarr/definitions"; -import { groupPermissions, permissionMatrix } from "@homarr/definitions"; +import { groupPermissions } from "@homarr/definitions"; import { getI18n, getScopedI18n } from "@homarr/translation/server"; import type { PermissionLabels } from "./_group-permission-form"; import { EffectivePermissionPreview, - MatrixRow, PermissionForm, + PermissionMatrix, PresetButtons, SaveAffix, } from "./_group-permission-form"; @@ -34,7 +34,7 @@ export default async function GroupPermissionsPage(props: GroupPermissionsPagePr const group = await api.group.getById({ id: params.id }); const t = await getI18n(); - const permissionLabels = await buildPermissionLabels(); + const { permissionLabels, permissionDescriptions } = await buildPermissionTextsAsync(); return ( @@ -44,11 +44,7 @@ export default async function GroupPermissionsPage(props: GroupPermissionsPagePr - - {objectKeys(permissionMatrix).map((category) => ( - - ))} - + @@ -59,8 +55,9 @@ export default async function GroupPermissionsPage(props: GroupPermissionsPagePr ); } -const buildPermissionLabels = async (): Promise => { +const buildPermissionTextsAsync = async () => { const permissionLabels: PermissionLabels = {}; + const permissionDescriptions: PermissionLabels = {}; for (const category of objectKeys(groupPermissions)) { const tItem = await getScopedI18n(`group.permission.${category}.item`); @@ -70,8 +67,9 @@ const buildPermissionLabels = async (): Promise => { suffixes.forEach((suffix) => { const key = (typeof item !== "boolean" ? `${category}-${suffix}` : "admin") as GroupPermissionKey; permissionLabels[key] = tItem(`${suffix}.label`); + permissionDescriptions[key] = tItem(`${suffix}.description`); }); } - return permissionLabels; + return { permissionLabels, permissionDescriptions }; }; diff --git a/packages/api/src/router/group.ts b/packages/api/src/router/group.ts index fc37ab766..201cc6b1a 100644 --- a/packages/api/src/router/group.ts +++ b/packages/api/src/router/group.ts @@ -9,7 +9,6 @@ import type { Database } from "@homarr/db"; import { and, eq, handleTransactionsAsync, like, not } from "@homarr/db"; import { getMaxGroupPositionAsync } from "@homarr/db/queries"; import { groupMembers, groupPermissions, groups, onboarding, users } from "@homarr/db/schema"; -import type { GroupPermissionKey } from "@homarr/definitions"; import { everyoneGroup } from "@homarr/definitions"; import { byIdSchema, paginatedSchema } from "@homarr/validation/common"; import { @@ -23,13 +22,6 @@ import { import { createTRPCRouter, onboardingProcedure, permissionRequiredProcedure, protectedProcedure } from "../trpc"; -const createPermissionParents = new Map([ - ["app-create", "app-modify-all"], - ["integration-create", "integration-full-all"], - ["board-create", "board-full-all"], - ["search-engine-create", "search-engine-modify-all"], -]); - export const groupRouter = createTRPCRouter({ getAll: permissionRequiredProcedure.requiresPermission("admin").query(async ({ ctx }) => { const dbGroups = await ctx.db.query.groups.findMany({ @@ -310,14 +302,9 @@ export const groupRouter = createTRPCRouter({ .mutation(async ({ input, ctx }) => { await throwIfGroupNotFoundAsync(ctx.db, input.groupId); - const permissions = [ - ...new Set( - input.permissions.flatMap((permission) => { - const parent = createPermissionParents.get(permission); - return parent ? [permission, parent] : [permission]; - }), - ), - ]; + // Stored exactly as selected - implied permissions are resolved at read time through + // getPermissionsWithChildren, so nothing is silently escalated here. + const permissions = [...new Set(input.permissions)]; await handleTransactionsAsync(ctx.db, { async handleAsync(db, schema) { diff --git a/packages/api/src/router/home.ts b/packages/api/src/router/home.ts index b30dab329..c9f29de41 100644 --- a/packages/api/src/router/home.ts +++ b/packages/api/src/router/home.ts @@ -123,12 +123,15 @@ export const homeRouter = createTRPCRouter({ }); } + // These mirror the access rules of /manage/integrations and /manage/apps exactly: a card is only + // shown when the page behind it is reachable, and it counts only what the user is allowed to see. const hasGlobalFullIntegrationAccess = ctx.session?.user.permissions.includes("integration-full-all") ?? false; + const canCreateIntegrations = ctx.session?.user.permissions.includes("integration-create") ?? false; const fullAccessIntegrationIds = hasGlobalFullIntegrationAccess || !ctx.session?.user ? [] : await getFullAccessIntegrationIdsAsync(ctx.db, ctx.session.user.id); - if (hasGlobalFullIntegrationAccess || fullAccessIntegrationIds.length > 0) { + if (hasGlobalFullIntegrationAccess || canCreateIntegrations || fullAccessIntegrationIds.length > 0) { statistics.push({ titleKey: "integration", subtitleKey: "resources", @@ -137,11 +140,12 @@ export const homeRouter = createTRPCRouter({ }); } - if (ctx.session?.user.permissions.includes("app-modify-all")) { + const canManageAllApps = ctx.session?.user.permissions.includes("app-modify-all") ?? false; + if (canManageAllApps || (ctx.session?.user.permissions.includes("app-create") ?? false)) { statistics.push({ titleKey: "app", subtitleKey: "resources", - count: await db.$count(apps), + count: canManageAllApps ? await db.$count(apps) : 0, path: "/manage/apps", }); } diff --git a/packages/api/src/router/test/group.spec.ts b/packages/api/src/router/test/group.spec.ts index 961b3b8c5..7dd50a68b 100644 --- a/packages/api/src/router/test/group.spec.ts +++ b/packages/api/src/router/test/group.spec.ts @@ -449,7 +449,7 @@ describe("savePermissions should save permissions for group", () => { expect(permissions.map(({ permission }) => permission)).toEqual(["integration-use-all", "board-full-all"]); }); - test("with a create permission it should also persist its parent permission", async () => { + test("with a standalone create permission it should not escalate to its parent permission", async () => { // Arrange const db = createDb(); const caller = groupRouter.createCaller({ db, deviceType: undefined, session: adminSession }); @@ -464,7 +464,7 @@ describe("savePermissions should save permissions for group", () => { // Act await caller.savePermissions({ groupId, - permissions: ["app-create"], + permissions: ["board-create"], }); // Assert @@ -472,37 +472,34 @@ describe("savePermissions should save permissions for group", () => { where: eq(groupPermissions.groupId, groupId), }); - expect(permissions.map(({ permission }) => permission).toSorted()).toEqual(["app-create", "app-modify-all"]); + expect(permissions.map(({ permission }) => permission)).toEqual(["board-create"]); }); - test("with an integration create permission it should persist its parent permission", async () => { - // Arrange - const db = createDb(); - const caller = groupRouter.createCaller({ db, deviceType: undefined, session: adminSession }); + test.each([["app-create"], ["integration-create"], ["search-engine-create"], ["media-upload"]] as const)( + "with %s alone it should persist exactly that permission", + async (permission) => { + // Arrange + const db = createDb(); + const caller = groupRouter.createCaller({ db, deviceType: undefined, session: adminSession }); - const groupId = createId(); - await db.insert(groups).values({ - id: groupId, - name: "Group", - position: 1, - }); + const groupId = createId(); + await db.insert(groups).values({ + id: groupId, + name: "Group", + position: 1, + }); - // Act - await caller.savePermissions({ - groupId, - permissions: ["integration-create"], - }); + // Act + await caller.savePermissions({ groupId, permissions: [permission] }); - // Assert - const permissions = await db.query.groupPermissions.findMany({ - where: eq(groupPermissions.groupId, groupId), - }); + // Assert + const permissions = await db.query.groupPermissions.findMany({ + where: eq(groupPermissions.groupId, groupId), + }); - expect(permissions.map(({ permission }) => permission).toSorted()).toEqual([ - "integration-create", - "integration-full-all", - ]); - }); + expect(permissions.map(({ permission: key }) => key)).toEqual([permission]); + }, + ); test("with a create permission and its parent it should persist exactly two rows", async () => { // Arrange @@ -531,6 +528,56 @@ describe("savePermissions should save permissions for group", () => { expect(permissions.map(({ permission }) => permission).toSorted()).toEqual(["app-create", "app-modify-all"]); }); + test("with duplicate permissions it should persist each permission only once", async () => { + // Arrange + const db = createDb(); + const caller = groupRouter.createCaller({ db, deviceType: undefined, session: adminSession }); + + const groupId = createId(); + await db.insert(groups).values({ + id: groupId, + name: "Group", + position: 1, + }); + + // Act + await caller.savePermissions({ + groupId, + permissions: ["board-view-all", "board-view-all", "board-modify-all"], + }); + + // Assert + const permissions = await db.query.groupPermissions.findMany({ + where: eq(groupPermissions.groupId, groupId), + }); + + expect(permissions.map(({ permission }) => permission).toSorted()).toEqual(["board-modify-all", "board-view-all"]); + }); + + test("saving the same permissions twice should be idempotent", async () => { + // Arrange + const db = createDb(); + const caller = groupRouter.createCaller({ db, deviceType: undefined, session: adminSession }); + + const groupId = createId(); + await db.insert(groups).values({ + id: groupId, + name: "Group", + position: 1, + }); + + // Act + await caller.savePermissions({ groupId, permissions: ["board-create", "media-upload"] }); + await caller.savePermissions({ groupId, permissions: ["board-create", "media-upload"] }); + + // Assert + const permissions = await db.query.groupPermissions.findMany({ + where: eq(groupPermissions.groupId, groupId), + }); + + expect(permissions.map(({ permission }) => permission).toSorted()).toEqual(["board-create", "media-upload"]); + }); + test("with an empty permission list it should clear all existing permissions", async () => { // Arrange const db = createDb(); diff --git a/packages/definitions/src/permissions.ts b/packages/definitions/src/permissions.ts index 3b43d59a5..c61963954 100644 --- a/packages/definitions/src/permissions.ts +++ b/packages/definitions/src/permissions.ts @@ -119,23 +119,53 @@ export const groupPermissionKeys = objectKeys(groupPermissions).reduce((acc, key }, [] as GroupPermissionKey[]); /** - * Ordered access levels per category used by the group permission matrix UI. - * Each level entry is a full {@link GroupPermissionKey}; higher levels imply the lower ones - * through {@link groupPermissionParents}. + * Describes how each permission category is presented in the group permission matrix UI. + * + * `levels` is an ordered escalation of access: picking level N grants every key up to N. + * `create` is orthogonal to `levels` - it is the "may add new resources" capability and can be + * granted on its own (e.g. "members may create their own boards but not see anyone else's"). + * Some levels already imply the create key through {@link groupPermissionParents}; use + * {@link isCreateImpliedByLevel} to detect that instead of hard-coding it. + * + * Together, `levels` and `create` cover every key in {@link groupPermissionKeys}, so the matrix + * is a lossless representation of a group's permissions. */ export const permissionMatrix = { - board: { levels: ["board-view-all", "board-modify-all", "board-full-all"] }, - app: { levels: ["app-use-all", "app-modify-all", "app-full-all"] }, - integration: { levels: ["integration-use-all", "integration-interact-all", "integration-full-all"] }, - "search-engine": { levels: ["search-engine-modify-all", "search-engine-full-all"] }, - media: { levels: ["media-view-all", "media-full-all"] }, - other: { levels: ["other-view-logs"] }, - admin: { levels: ["admin"] }, -} as const satisfies Record; + board: { levels: ["board-view-all", "board-modify-all", "board-full-all"], create: "board-create" }, + app: { levels: ["app-use-all", "app-modify-all", "app-full-all"], create: "app-create" }, + integration: { + levels: ["integration-use-all", "integration-interact-all", "integration-full-all"], + create: "integration-create", + }, + "search-engine": { levels: ["search-engine-modify-all", "search-engine-full-all"], create: "search-engine-create" }, + media: { levels: ["media-view-all", "media-full-all"], create: "media-upload" }, + other: { levels: ["other-view-logs"], create: null }, + admin: { levels: ["admin"], create: null }, +} as const satisfies Record; export type PermissionMatrixCategory = keyof typeof permissionMatrix; -export type PermissionMatrixState = Record; +export interface PermissionMatrixCategoryState { + /** 0 = no access, N = every key up to `permissionMatrix[category].levels[N - 1]` */ + level: number; + /** Whether the category's create/upload key is granted */ + create: boolean; +} + +export type PermissionMatrixState = Record; + +/** + * Whether the given level already grants the category's create key through permission inheritance. + * For example board level 3 (`board-full-all`) implies `board-create`. + */ +export const isCreateImpliedByLevel = (category: PermissionMatrixCategory, level: number) => { + const createKey = permissionMatrix[category].create; + if (createKey === null || level <= 0) { + return false; + } + + return getPermissionsWithChildren([...permissionMatrix[category].levels.slice(0, level)]).includes(createKey); +}; export const permissionsToMatrixState = (permissions: GroupPermissionKey[]): PermissionMatrixState => { const permissionSet = new Set(permissions); @@ -147,7 +177,11 @@ export const permissionsToMatrixState = (permissions: GroupPermissionKey[]): Per level = index + 1; } }); - acc[category] = { level }; + const createKey = permissionMatrix[category].create; + acc[category] = { + level, + create: createKey !== null && (permissionSet.has(createKey) || isCreateImpliedByLevel(category, level)), + }; return acc; }, {} as PermissionMatrixState); }; @@ -155,6 +189,12 @@ export const permissionsToMatrixState = (permissions: GroupPermissionKey[]): Per export const matrixStateToPermissions = (state: PermissionMatrixState): GroupPermissionKey[] => { return objectKeys(permissionMatrix).reduce((acc, category) => { acc.push(...permissionMatrix[category].levels.slice(0, state[category].level)); + // Skip the create key when the chosen level already implies it, so the stored permissions + // stay minimal and a state -> permissions -> state round-trip is exact. + const createKey = permissionMatrix[category].create; + if (createKey !== null && state[category].create && !isCreateImpliedByLevel(category, state[category].level)) { + acc.push(createKey); + } return acc; }, [] as GroupPermissionKey[]); }; diff --git a/packages/definitions/src/test/permissions.spec.ts b/packages/definitions/src/test/permissions.spec.ts index 5c8da676a..d64093feb 100644 --- a/packages/definitions/src/test/permissions.spec.ts +++ b/packages/definitions/src/test/permissions.spec.ts @@ -1,13 +1,27 @@ import { describe, expect, test } from "vitest"; -import type { GroupPermissionKey } from "../permissions"; +import type { GroupPermissionKey, PermissionMatrixState } from "../permissions"; import { getPermissionsWithChildren, getPermissionsWithParents, + groupPermissionKeys, + isCreateImpliedByLevel, matrixStateToPermissions, + permissionMatrix, permissionsToMatrixState, } from "../permissions"; +const emptyState = (overrides: Partial = {}): PermissionMatrixState => ({ + board: { level: 0, create: false }, + app: { level: 0, create: false }, + integration: { level: 0, create: false }, + "search-engine": { level: 0, create: false }, + media: { level: 0, create: false }, + other: { level: 0, create: false }, + admin: { level: 0, create: false }, + ...overrides, +}); + describe("getPermissionsWithParents should return the correct permissions", () => { test.each([ [["board-view-all"], ["board-view-all", "board-modify-all", "board-full-all", "admin"]], @@ -51,10 +65,23 @@ describe("getPermissionsWithChildren should return the correct permissions", () }); }); +describe("the matrix must be able to represent every permission", () => { + test("every group permission key is covered by exactly one category", () => { + const covered = Object.values(permissionMatrix).flatMap(({ levels, create }) => [ + ...levels, + ...(create === null ? [] : [create]), + ]); + expect(covered.toSorted()).toEqual(groupPermissionKeys.toSorted()); + }); + + test.each(groupPermissionKeys.map((key) => [key]))("expect %s to survive a round-trip on its own", (key) => { + expect(matrixStateToPermissions(permissionsToMatrixState([key]))).toContain(key); + }); +}); + describe("permissionsToMatrixState should resolve the highest present level per category", () => { test("resolves the highest present level key", () => { - const state = permissionsToMatrixState(["board-view-all", "board-modify-all"]); - expect(state.board.level).toBe(2); + expect(permissionsToMatrixState(["board-view-all", "board-modify-all"]).board.level).toBe(2); }); test("resolves the admin category", () => { @@ -63,35 +90,74 @@ describe("permissionsToMatrixState should resolve the highest present level per test("resolves an empty selection to all zero", () => { const state = permissionsToMatrixState([]); - expect(Object.values(state).every(({ level }) => level === 0)).toBe(true); + expect(Object.values(state).every(({ level, create }) => level === 0 && !create)).toBe(true); }); + + test("resolves a standalone create key without granting any level", () => { + expect(permissionsToMatrixState(["board-create"]).board).toEqual({ level: 0, create: true }); + }); + + test("marks create as granted when the level implies it", () => { + expect(permissionsToMatrixState(["board-full-all"]).board.create).toBe(true); + expect(permissionsToMatrixState(["app-modify-all"]).app.create).toBe(true); + expect(permissionsToMatrixState(["media-full-all"]).media.create).toBe(true); + }); + + test("does not mark create as granted for a level that does not imply it", () => { + expect(permissionsToMatrixState(["board-modify-all"]).board.create).toBe(false); + expect(permissionsToMatrixState(["integration-interact-all"]).integration.create).toBe(false); + }); +}); + +describe("isCreateImpliedByLevel", () => { + test.each([ + ["board", 0, false], + ["board", 1, false], + ["board", 2, false], + ["board", 3, true], + ["app", 1, false], + ["app", 2, true], + ["integration", 2, false], + ["integration", 3, true], + ["search-engine", 1, true], + ["media", 1, false], + ["media", 2, true], + ["other", 1, false], + ["admin", 1, false], + ] satisfies [keyof typeof permissionMatrix, number, boolean][])( + "expect %s at level %i to imply create: %s", + (category, level, expected) => { + expect(isCreateImpliedByLevel(category, level)).toBe(expected); + }, + ); }); describe("matrixStateToPermissions should expand levels into permission keys", () => { test("expands a level into the ordered level keys", () => { - const permissions = matrixStateToPermissions({ - board: { level: 2 }, - app: { level: 0 }, - integration: { level: 0 }, - "search-engine": { level: 0 }, - media: { level: 0 }, - other: { level: 0 }, - admin: { level: 0 }, - }); - expect(permissions).toEqual(["board-view-all", "board-modify-all"]); + expect(matrixStateToPermissions(emptyState({ board: { level: 2, create: false } }))).toEqual([ + "board-view-all", + "board-modify-all", + ]); }); test("expands the admin category into the admin key", () => { - const permissions = matrixStateToPermissions({ - board: { level: 0 }, - app: { level: 0 }, - integration: { level: 0 }, - "search-engine": { level: 0 }, - media: { level: 0 }, - other: { level: 0 }, - admin: { level: 1 }, - }); - expect(permissions).toEqual(["admin"]); + expect(matrixStateToPermissions(emptyState({ admin: { level: 1, create: false } }))).toEqual(["admin"]); + }); + + test("emits a standalone create key without any level", () => { + expect(matrixStateToPermissions(emptyState({ board: { level: 0, create: true } }))).toEqual(["board-create"]); + }); + + test("emits media-upload as the create key of the media category", () => { + expect(matrixStateToPermissions(emptyState({ media: { level: 0, create: true } }))).toEqual(["media-upload"]); + }); + + test("omits a create key that the chosen level already implies", () => { + expect(matrixStateToPermissions(emptyState({ board: { level: 3, create: true } }))).toEqual([ + "board-view-all", + "board-modify-all", + "board-full-all", + ]); }); }); @@ -111,6 +177,10 @@ describe("matrix helpers should round-trip stably", () => { "other-view-logs", ], ], + [["board-create"]], + [["media-upload"]], + [["board-create", "app-create", "integration-create", "search-engine-create", "media-upload"]], + [["board-create", "board-view-all"]], ] satisfies [GroupPermissionKey[]][])("expect %s to round-trip", (permissions) => { const roundTripped = matrixStateToPermissions(permissionsToMatrixState(permissions)); expect(roundTripped).toEqual(expect.arrayContaining(permissions)); @@ -118,7 +188,10 @@ describe("matrix helpers should round-trip stably", () => { }); test("expect a single higher-level key to expand to its full set on round-trip", () => { - const roundTripped = matrixStateToPermissions(permissionsToMatrixState(["board-full-all"])); - expect(roundTripped).toEqual(["board-view-all", "board-modify-all", "board-full-all"]); + expect(matrixStateToPermissions(permissionsToMatrixState(["board-full-all"]))).toEqual([ + "board-view-all", + "board-modify-all", + "board-full-all", + ]); }); }); diff --git a/packages/translation/src/lang/en.json b/packages/translation/src/lang/en.json index 464dfb114..b67a0f4b0 100644 --- a/packages/translation/src/lang/en.json +++ b/packages/translation/src/lang/en.json @@ -655,20 +655,32 @@ } }, "matrix": { - "none": "None", - "impliedHint": "also grants: {permissions}", - "grantedViaAdmin": "Granted via Administrator", + "none": "No access", + "impliedHint": "Also grants: {permissions}", + "grantedViaAdmin": "Administrators already have this. Set Admin to “No access” to configure it separately.", "preset": { "label": "Presets", "viewer": "Viewer", + "viewerDescription": "Read-only: view boards and medias, use apps and integrations that already exist.", "editor": "Editor", + "editorDescription": "Day-to-day management: create and edit boards, apps, integrations and search engines.", "admin": "Admin", + "adminDescription": "Unrestricted access to every feature and setting, including user management.", "custom": "Custom" }, "effective": { "title": "Effective permissions", - "description": "Everything members of this group will be able to do, including implied permissions", + "description": "The full, de-duplicated set of actions members of this group can perform, including permissions granted indirectly.", "empty": "No permissions selected" + }, + "noneDescription": "Members get no access to this area.", + "createIncluded": "Already included in “{level}”", + "level": { + "view": "View", + "use": "Use", + "interact": "Interact", + "modify": "Modify", + "full": "Full" } } }, @@ -796,7 +808,9 @@ "action": "Create your first app", "filteredTitle": "No apps match your search", "filteredDescription": "No apps match \"{search}\". Try another name or URL.", - "clearSearch": "Clear search" + "clearSearch": "Clear search", + "createOnlyTitle": "You can only add apps", + "createOnlyDescription": "Your permissions let you create apps but not browse existing ones. Ask an administrator for app management access to see the full list." } }, "create": { @@ -1598,7 +1612,7 @@ "title": "Integrations", "ariaLabel": "Configured integrations", "search": "Search integrations", - "scopedNote": "Showing only integrations you have full access to", + "scopedNote": "Showing only the integrations you are allowed to manage.", "action": { "edit": "Edit {name}", "delete": "Delete {name}" @@ -1610,8 +1624,8 @@ "filteredTitle": "No integrations match your search", "filteredDescription": "No integrations match \"{search}\". Try another name, type, or URL.", "clearSearch": "Clear search", - "noPermissionTitle": "No manageable integrations", - "noPermissionDescription": "You don't have full management access to any integrations. Ask your administrator." + "createOnlyTitle": "You can only add integrations", + "createOnlyDescription": "Your permissions let you create integrations but not manage existing ones. Ask an administrator for access to the ones you need." } }, "create": {