fix(permissions): keep create permissions grantable and stop silent escalation
The matrix could not represent board-create, app-create, integration-create, search-engine-create or media-upload, and savePermissions force-bundled every create key with its modify/full parent. Together that meant a group with board-create was silently upgraded to board-full-all on the next save, and the common "members may create their own boards" setup became unreachable. - permissionMatrix models create/upload as an orthogonal capability next to the ordered access levels, so all 20 permission keys round-trip losslessly - savePermissions stores exactly what was selected - the matrix UI gets a per-category create checkbox, auto-checked and disabled when the chosen level already implies it - a management section is reachable when the user can do at least one thing in it; nav, pages and stat cards now share one access helper
This commit is contained in:
12 files changed
+534
-177
No files matched your search
@@ -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<GroupPermissionKey, GroupPermissionKey>([
|
||||
["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) {
|
||||
|
||||
@@ -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",
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in new issue
Block a user