feat: add more group permissions (#1453)

* feat: add more group permissions * feat: restrict access with app permissions * feat: restrict access with search-engine permissions * feat: restrict access with media permissions * refactor: remove permissions for users, groups and invites * test: adjust app router tests with app permissions * fix: integration page accessible without session * fix: search for users, groups and integrations shown to unauthenticated users * chore: address pull request feedback
2026-02-27 08:50:56 +01:00 · 2024-11-17 21:31:25 +01:00
parent 879aa1152f
commit 0ee343b99e
31 changed files with 575 additions and 208 deletions
--- a/packages/api/src/router/app.ts
+++ b/packages/api/src/router/app.ts
@@ -4,10 +4,11 @@ import { asc, createId, eq, inArray, like } from "@homarr/db";
 import { apps } from "@homarr/db/schema/sqlite";
 import { validation, z } from "@homarr/validation";

-import { createTRPCRouter, protectedProcedure, publicProcedure } from "../trpc";
+import { createTRPCRouter, permissionRequiredProcedure, protectedProcedure, publicProcedure } from "../trpc";
+import { canUserSeeAppAsync } from "./app/app-access-control";

 export const appRouter = createTRPCRouter({
-  all: publicProcedure
+  all: protectedProcedure
    .input(z.void())
    .output(
      z.array(
@@ -26,7 +27,7 @@ export const appRouter = createTRPCRouter({
        orderBy: asc(apps.name),
      });
    }),
-  search: publicProcedure
+  search: protectedProcedure
    .input(z.object({ query: z.string(), limit: z.number().min(1).max(100).default(10) }))
    .output(
      z.array(
@@ -47,7 +48,7 @@ export const appRouter = createTRPCRouter({
        limit: input.limit,
      });
    }),
-  selectable: publicProcedure
+  selectable: protectedProcedure
    .input(z.void())
    .output(
      z.array(
@@ -104,14 +105,23 @@ export const appRouter = createTRPCRouter({
        });
      }

+      const canUserSeeApp = await canUserSeeAppAsync(ctx.session?.user ?? null, app.id);
+      if (!canUserSeeApp) {
+        throw new TRPCError({
+          code: "NOT_FOUND",
+          message: "App not found",
+        });
+      }
+
      return app;
    }),
-  byIds: publicProcedure.input(z.array(z.string())).query(async ({ ctx, input }) => {
+  byIds: protectedProcedure.input(z.array(z.string())).query(async ({ ctx, input }) => {
    return await ctx.db.query.apps.findMany({
      where: inArray(apps.id, input),
    });
  }),
-  create: protectedProcedure
+  create: permissionRequiredProcedure
+    .requiresPermission("app-create")
    .input(validation.app.manage)
    .output(z.void())
    .meta({ openapi: { method: "POST", path: "/api/apps", tags: ["apps"], protect: true } })
@@ -124,29 +134,33 @@ export const appRouter = createTRPCRouter({
        href: input.href,
      });
    }),
-  update: protectedProcedure.input(validation.app.edit).mutation(async ({ ctx, input }) => {
-    const app = await ctx.db.query.apps.findFirst({
-      where: eq(apps.id, input.id),
-    });
-
-    if (!app) {
-      throw new TRPCError({
-        code: "NOT_FOUND",
-        message: "App not found",
+  update: permissionRequiredProcedure
+    .requiresPermission("app-modify-all")
+    .input(validation.app.edit)
+    .mutation(async ({ ctx, input }) => {
+      const app = await ctx.db.query.apps.findFirst({
+        where: eq(apps.id, input.id),
      });
-    }

-    await ctx.db
-      .update(apps)
-      .set({
-        name: input.name,
-        description: input.description,
-        iconUrl: input.iconUrl,
-        href: input.href,
-      })
-      .where(eq(apps.id, input.id));
-  }),
-  delete: protectedProcedure
+      if (!app) {
+        throw new TRPCError({
+          code: "NOT_FOUND",
+          message: "App not found",
+        });
+      }
+
+      await ctx.db
+        .update(apps)
+        .set({
+          name: input.name,
+          description: input.description,
+          iconUrl: input.iconUrl,
+          href: input.href,
+        })
+        .where(eq(apps.id, input.id));
+    }),
+  delete: permissionRequiredProcedure
+    .requiresPermission("app-full-all")
    .output(z.void())
    .meta({ openapi: { method: "DELETE", path: "/api/apps/{id}", tags: ["apps"], protect: true } })
    .input(validation.common.byId)
--- a/packages/api/src/router/app/app-access-control.ts
+++ b/packages/api/src/router/app/app-access-control.ts
@@ -0,0 +1,50 @@
+import SuperJSON from "superjson";
+
+import type { Session } from "@homarr/auth";
+import { db, eq, or } from "@homarr/db";
+import { items } from "@homarr/db/schema/sqlite";
+
+import type { WidgetComponentProps } from "../../../../widgets/src";
+
+export const canUserSeeAppAsync = async (user: Session["user"] | null, appId: string) => {
+  return await canUserSeeAppsAsync(user, [appId]);
+};
+
+export const canUserSeeAppsAsync = async (user: Session["user"] | null, appIds: string[]) => {
+  if (user) return true;
+
+  const appIdsOnPublicBoards = await getAllAppIdsOnPublicBoardsAsync();
+  return appIds.every((appId) => appIdsOnPublicBoards.includes(appId));
+};
+
+const getAllAppIdsOnPublicBoardsAsync = async () => {
+  const itemsWithApps = await db.query.items.findMany({
+    where: or(eq(items.kind, "app"), eq(items.kind, "bookmarks")),
+    with: {
+      section: {
+        columns: {}, // Nothing
+        with: {
+          board: {
+            columns: {
+              isPublic: true,
+            },
+          },
+        },
+      },
+    },
+  });
+
+  return itemsWithApps
+    .filter((item) => item.section.board.isPublic)
+    .flatMap((item) => {
+      if (item.kind === "app") {
+        const parsedOptions = SuperJSON.parse<WidgetComponentProps<"app">["options"]>(item.options);
+        return [parsedOptions.appId];
+      } else if (item.kind === "bookmarks") {
+        const parsedOptions = SuperJSON.parse<WidgetComponentProps<"bookmarks">["options"]>(item.options);
+        return parsedOptions.items;
+      }
+
+      throw new Error("Failed to get app ids from board. Invalid item kind: 'test'");
+    });
+};
--- a/packages/api/src/router/log.ts
+++ b/packages/api/src/router/log.ts
@@ -7,7 +7,7 @@ import { loggingChannel } from "@homarr/redis";
 import { createTRPCRouter, permissionRequiredProcedure } from "../trpc";

 export const logRouter = createTRPCRouter({
-  subscribe: permissionRequiredProcedure.requiresPermission("admin").subscription(() => {
+  subscribe: permissionRequiredProcedure.requiresPermission("other-view-logs").subscription(() => {
    return observable<LoggerMessage>((emit) => {
      const unsubscribe = loggingChannel.subscribe((data) => {
        emit.next(data);
--- a/packages/api/src/router/medias/media-router.ts
+++ b/packages/api/src/router/medias/media-router.ts
@@ -4,7 +4,7 @@ import { and, createId, desc, eq, like } from "@homarr/db";
 import { medias } from "@homarr/db/schema/sqlite";
 import { validation, z } from "@homarr/validation";

-import { createTRPCRouter, protectedProcedure } from "../../trpc";
+import { createTRPCRouter, permissionRequiredProcedure, protectedProcedure } from "../../trpc";

 export const mediaRouter = createTRPCRouter({
  getPaginated: protectedProcedure
@@ -14,7 +14,7 @@ export const mediaRouter = createTRPCRouter({
      ),
    )
    .query(async ({ ctx, input }) => {
-      const includeFromAllUsers = ctx.session.user.permissions.includes("admin") && input.includeFromAllUsers;
+      const includeFromAllUsers = ctx.session.user.permissions.includes("media-view-all") && input.includeFromAllUsers;

      const where = and(
        input.search.length >= 1 ? like(medias.name, `%${input.search}%`) : undefined,
@@ -46,20 +46,23 @@ export const mediaRouter = createTRPCRouter({
        totalCount,
      };
    }),
-  uploadMedia: protectedProcedure.input(validation.media.uploadMedia).mutation(async ({ ctx, input }) => {
-    const content = Buffer.from(await input.file.arrayBuffer());
-    const id = createId();
-    await ctx.db.insert(medias).values({
-      id,
-      creatorId: ctx.session.user.id,
-      content,
-      size: input.file.size,
-      contentType: input.file.type,
-      name: input.file.name,
-    });
+  uploadMedia: permissionRequiredProcedure
+    .requiresPermission("media-upload")
+    .input(validation.media.uploadMedia)
+    .mutation(async ({ ctx, input }) => {
+      const content = Buffer.from(await input.file.arrayBuffer());
+      const id = createId();
+      await ctx.db.insert(medias).values({
+        id,
+        creatorId: ctx.session.user.id,
+        content,
+        size: input.file.size,
+        contentType: input.file.type,
+        name: input.file.name,
+      });

-    return id;
-  }),
+      return id;
+    }),
  deleteMedia: protectedProcedure.input(validation.common.byId).mutation(async ({ ctx, input }) => {
    const dbMedia = await ctx.db.query.medias.findFirst({
      where: eq(medias.id, input.id),
@@ -75,8 +78,8 @@ export const mediaRouter = createTRPCRouter({
      });
    }

-    // Only allow admins and the creator of the media to delete it
-    if (!ctx.session.user.permissions.includes("admin") && ctx.session.user.id !== dbMedia.creatorId) {
+    // Only allow users with media-full-all permission and the creator of the media to delete it
+    if (!ctx.session.user.permissions.includes("media-full-all") && ctx.session.user.id !== dbMedia.creatorId) {
      throw new TRPCError({
        code: "FORBIDDEN",
        message: "You don't have permission to delete this media",
--- a/packages/api/src/router/search-engine/search-engine-router.ts
+++ b/packages/api/src/router/search-engine/search-engine-router.ts
@@ -4,7 +4,7 @@ import { createId, eq, like, sql } from "@homarr/db";
 import { searchEngines } from "@homarr/db/schema/sqlite";
 import { validation } from "@homarr/validation";

-import { createTRPCRouter, protectedProcedure } from "../../trpc";
+import { createTRPCRouter, permissionRequiredProcedure, protectedProcedure } from "../../trpc";

 export const searchEngineRouter = createTRPCRouter({
  getPaginated: protectedProcedure.input(validation.common.paginated).query(async ({ input, ctx }) => {
@@ -59,43 +59,52 @@ export const searchEngineRouter = createTRPCRouter({
      limit: input.limit,
    });
  }),
-  create: protectedProcedure.input(validation.searchEngine.manage).mutation(async ({ ctx, input }) => {
-    await ctx.db.insert(searchEngines).values({
-      id: createId(),
-      name: input.name,
-      short: input.short.toLowerCase(),
-      iconUrl: input.iconUrl,
-      urlTemplate: "urlTemplate" in input ? input.urlTemplate : null,
-      description: input.description,
-      type: input.type,
-      integrationId: "integrationId" in input ? input.integrationId : null,
-    });
-  }),
-  update: protectedProcedure.input(validation.searchEngine.edit).mutation(async ({ ctx, input }) => {
-    const searchEngine = await ctx.db.query.searchEngines.findFirst({
-      where: eq(searchEngines.id, input.id),
-    });
-
-    if (!searchEngine) {
-      throw new TRPCError({
-        code: "NOT_FOUND",
-        message: "Search engine not found",
-      });
-    }
-
-    await ctx.db
-      .update(searchEngines)
-      .set({
+  create: permissionRequiredProcedure
+    .requiresPermission("search-engine-create")
+    .input(validation.searchEngine.manage)
+    .mutation(async ({ ctx, input }) => {
+      await ctx.db.insert(searchEngines).values({
+        id: createId(),
        name: input.name,
+        short: input.short.toLowerCase(),
        iconUrl: input.iconUrl,
        urlTemplate: "urlTemplate" in input ? input.urlTemplate : null,
        description: input.description,
-        integrationId: "integrationId" in input ? input.integrationId : null,
        type: input.type,
-      })
-      .where(eq(searchEngines.id, input.id));
-  }),
-  delete: protectedProcedure.input(validation.common.byId).mutation(async ({ ctx, input }) => {
-    await ctx.db.delete(searchEngines).where(eq(searchEngines.id, input.id));
-  }),
+        integrationId: "integrationId" in input ? input.integrationId : null,
+      });
+    }),
+  update: permissionRequiredProcedure
+    .requiresPermission("search-engine-modify-all")
+    .input(validation.searchEngine.edit)
+    .mutation(async ({ ctx, input }) => {
+      const searchEngine = await ctx.db.query.searchEngines.findFirst({
+        where: eq(searchEngines.id, input.id),
+      });
+
+      if (!searchEngine) {
+        throw new TRPCError({
+          code: "NOT_FOUND",
+          message: "Search engine not found",
+        });
+      }
+
+      await ctx.db
+        .update(searchEngines)
+        .set({
+          name: input.name,
+          iconUrl: input.iconUrl,
+          urlTemplate: "urlTemplate" in input ? input.urlTemplate : null,
+          description: input.description,
+          integrationId: "integrationId" in input ? input.integrationId : null,
+          type: input.type,
+        })
+        .where(eq(searchEngines.id, input.id));
+    }),
+  delete: permissionRequiredProcedure
+    .requiresPermission("search-engine-full-all")
+    .input(validation.common.byId)
+    .mutation(async ({ ctx, input }) => {
+      await ctx.db.delete(searchEngines).where(eq(searchEngines.id, input.id));
+    }),
 });
--- a/packages/api/src/router/test/app.spec.ts
+++ b/packages/api/src/router/test/app.spec.ts
@@ -5,23 +5,26 @@ import type { Session } from "@homarr/auth";
 import { createId } from "@homarr/db";
 import { apps } from "@homarr/db/schema/sqlite";
 import { createDb } from "@homarr/db/test";
+import type { GroupPermissionKey } from "@homarr/definitions";

 import { appRouter } from "../app";
+import * as appAccessControl from "../app/app-access-control";

 // Mock the auth module to return an empty session
 vi.mock("@homarr/auth", () => ({ auth: () => ({}) as Session }));

-const defaultSession: Session = {
-  user: { id: createId(), permissions: [], colorScheme: "light" },
+const createDefaultSession = (permissions: GroupPermissionKey[] = []): Session => ({
+  user: { id: createId(), permissions, colorScheme: "light" },
  expires: new Date().toISOString(),
-};
+});

 describe("all should return all apps", () => {
-  test("should return all apps", async () => {
+  test("should return all apps with session", async () => {
+    // Arrange
    const db = createDb();
    const caller = appRouter.createCaller({
      db,
-      session: null,
+      session: createDefaultSession(),
    });

    await db.insert(apps).values([
@@ -48,15 +51,30 @@ describe("all should return all apps", () => {
    expect(result[1]!.href).toBeNull();
    expect(result[1]!.description).toBeNull();
  });
+  test("should throw UNAUTHORIZED if the user is not authenticated", async () => {
+    // Arrange
+    const caller = appRouter.createCaller({
+      db: createDb(),
+      session: null,
+    });
+
+    // Act
+    const actAsync = async () => await caller.all();
+
+    // Assert
+    await expect(actAsync()).rejects.toThrow("UNAUTHORIZED");
+  });
 });

 describe("byId should return an app by id", () => {
-  test("should return an app by id", async () => {
+  test("should return an app by id when canUserSeeAppAsync returns true", async () => {
+    // Arrange
    const db = createDb();
    const caller = appRouter.createCaller({
      db,
      session: null,
    });
+    vi.spyOn(appAccessControl, "canUserSeeAppAsync").mockReturnValue(Promise.resolve(true));

    await db.insert(apps).values([
      {
@@ -73,28 +91,61 @@ describe("byId should return an app by id", () => {
      },
    ]);

+    // Act
    const result = await caller.byId({ id: "2" });
+
+    // Assert
    expect(result.name).toBe("Mantine");
  });

+  test("should throw NOT_FOUND error when canUserSeeAppAsync returns false", async () => {
+    // Arrange
+    const db = createDb();
+    const caller = appRouter.createCaller({
+      db,
+      session: null,
+    });
+    await db.insert(apps).values([
+      {
+        id: "2",
+        name: "Mantine",
+        description: "React components and hooks library",
+        iconUrl: "https://mantine.dev/favicon.svg",
+        href: "https://mantine.dev",
+      },
+    ]);
+    vi.spyOn(appAccessControl, "canUserSeeAppAsync").mockReturnValue(Promise.resolve(false));
+
+    // Act
+    const actAsync = async () => await caller.byId({ id: "2" });
+
+    // Assert
+    await expect(actAsync()).rejects.toThrow("App not found");
+  });
+
  test("should throw an error if the app does not exist", async () => {
+    // Arrange
    const db = createDb();
    const caller = appRouter.createCaller({
      db,
      session: null,
    });

+    // Act
    const actAsync = async () => await caller.byId({ id: "2" });
+
+    // Assert
    await expect(actAsync()).rejects.toThrow("App not found");
  });
 });

 describe("create should create a new app with all arguments", () => {
  test("should create a new app", async () => {
+    // Arrange
    const db = createDb();
    const caller = appRouter.createCaller({
      db,
-      session: defaultSession,
+      session: createDefaultSession(["app-create"]),
    });
    const input = {
      name: "Mantine",
@@ -103,8 +154,10 @@ describe("create should create a new app with all arguments", () => {
      href: "https://mantine.dev",
    };

+    // Act
    await caller.create(input);

+    // Assert
    const dbApp = await db.query.apps.findFirst();
    expect(dbApp).toBeDefined();
    expect(dbApp!.name).toBe(input.name);
@@ -114,10 +167,11 @@ describe("create should create a new app with all arguments", () => {
  });

  test("should create a new app only with required arguments", async () => {
+    // Arrange
    const db = createDb();
    const caller = appRouter.createCaller({
      db,
-      session: defaultSession,
+      session: createDefaultSession(["app-create"]),
    });
    const input = {
      name: "Mantine",
@@ -126,8 +180,10 @@ describe("create should create a new app with all arguments", () => {
      href: null,
    };

+    // Act
    await caller.create(input);

+    // Assert
    const dbApp = await db.query.apps.findFirst();
    expect(dbApp).toBeDefined();
    expect(dbApp!.name).toBe(input.name);
@@ -139,10 +195,11 @@ describe("create should create a new app with all arguments", () => {

 describe("update should update an app", () => {
  test("should update an app", async () => {
+    // Arrange
    const db = createDb();
    const caller = appRouter.createCaller({
      db,
-      session: defaultSession,
+      session: createDefaultSession(["app-modify-all"]),
    });

    const appId = createId();
@@ -162,8 +219,10 @@ describe("update should update an app", () => {
      href: "https://mantine.dev",
    };

+    // Act
    await caller.update(input);

+    // Assert
    const dbApp = await db.query.apps.findFirst();

    expect(dbApp).toBeDefined();
@@ -174,12 +233,14 @@ describe("update should update an app", () => {
  });

  test("should throw an error if the app does not exist", async () => {
+    // Arrange
    const db = createDb();
    const caller = appRouter.createCaller({
      db,
-      session: defaultSession,
+      session: createDefaultSession(["app-modify-all"]),
    });

+    // Act
    const actAsync = async () =>
      await caller.update({
        id: createId(),
@@ -188,16 +249,19 @@ describe("update should update an app", () => {
        description: null,
        href: null,
      });
+
+    // Assert
    await expect(actAsync()).rejects.toThrow("App not found");
  });
 });

 describe("delete should delete an app", () => {
  test("should delete an app", async () => {
+    // Arrange
    const db = createDb();
    const caller = appRouter.createCaller({
      db,
-      session: defaultSession,
+      session: createDefaultSession(["app-full-all"]),
    });

    const appId = createId();
@@ -207,8 +271,10 @@ describe("delete should delete an app", () => {
      iconUrl: "https://mantine.dev/favicon.svg",
    });

+    // Act
    await caller.delete({ id: appId });

+    // Assert
    const dbApp = await db.query.apps.findFirst();
    expect(dbApp).toBeUndefined();
  });