From 64e3bf48b9343987fe6f005ce77f32053e82c8d1 Mon Sep 17 00:00:00 2001 From: Nico <47644445+nicotsx@users.noreply.github.com> Date: Sun, 14 Jun 2026 20:16:34 +0200 Subject: [PATCH] refactor(auth): enforce named permissions on server routes (#984) --- .../auth.controller.security.test.ts | 4 +- app/server/modules/auth/auth.controller.ts | 84 +++++++++++-------- .../sso/__tests__/sso.registration.test.ts | 26 +++++- .../sso/middlewares/authorize-registration.ts | 22 ++++- app/server/modules/sso/sso.controller.ts | 18 ++-- .../modules/system/system.controller.ts | 7 +- 6 files changed, 112 insertions(+), 49 deletions(-) diff --git a/app/server/modules/auth/__tests__/auth.controller.security.test.ts b/app/server/modules/auth/__tests__/auth.controller.security.test.ts index fcd856a9..11f0f404 100644 --- a/app/server/modules/auth/__tests__/auth.controller.security.test.ts +++ b/app/server/modules/auth/__tests__/auth.controller.security.test.ts @@ -70,7 +70,7 @@ describe("auth controller security", () => { } }); - describe("org admin endpoints - require requireAuth + requireOrgAdmin", () => { + describe("organization management endpoints - require named organization permissions", () => { const orgAdminEndpoints = [ { method: "GET", path: "/api/v1/auth/sso-settings" }, { method: "DELETE", path: "/api/v1/auth/sso-providers/test-provider" }, @@ -141,7 +141,7 @@ describe("auth controller security", () => { }); }); - describe("global admin endpoints - require requireAuth + requireAdmin", () => { + describe("instance user management endpoints - require instanceUsers.manage", () => { const adminEndpoints = [ { method: "GET", path: "/api/v1/auth/admin-users" }, { method: "DELETE", path: "/api/v1/auth/admin-users/test-user/accounts/test-account" }, diff --git a/app/server/modules/auth/auth.controller.ts b/app/server/modules/auth/auth.controller.ts index 21efd34f..23db4ee4 100644 --- a/app/server/modules/auth/auth.controller.ts +++ b/app/server/modules/auth/auth.controller.ts @@ -15,7 +15,7 @@ import { removeOrgMemberDto, } from "./auth.dto"; import { authService } from "./auth.service"; -import { requireAdmin, requireAuth, requireOrgAdmin } from "./auth.middleware"; +import { requireAuth, requirePermission } from "./auth.middleware"; import { auth } from "~/server/lib/auth"; export const authController = new Hono() @@ -23,7 +23,7 @@ export const authController = new Hono() const hasUsers = await authService.hasUsers(); return c.json({ hasUsers }); }) - .get("/admin-users", requireAuth, requireAdmin, getAdminUsersDto, async (c) => { + .get("/admin-users", requireAuth, requirePermission("instanceUsers.manage"), getAdminUsersDto, async (c) => { const headers = c.req.raw.headers; const usersData = await auth.api.listUsers({ @@ -46,27 +46,39 @@ export const authController = new Hono() total: usersData.total, }); }) - .delete("/admin-users/:userId/accounts/:accountId", requireAuth, requireAdmin, deleteUserAccountDto, async (c) => { - const userId = c.req.param("userId"); - const accountId = c.req.param("accountId"); - const result = await authService.deleteUserAccount(userId, accountId); + .delete( + "/admin-users/:userId/accounts/:accountId", + requireAuth, + requirePermission("instanceUsers.manage"), + deleteUserAccountDto, + async (c) => { + const userId = c.req.param("userId"); + const accountId = c.req.param("accountId"); + const result = await authService.deleteUserAccount(userId, accountId); - if (result.lastAccount) { - return c.json({ message: "Cannot delete the last account of a user" }, 409); - } + if (result.lastAccount) { + return c.json({ message: "Cannot delete the last account of a user" }, 409); + } - if (result.notFound) { - return c.json({ message: "Account not found" }, 404); - } + if (result.notFound) { + return c.json({ message: "Account not found" }, 404); + } - return c.json({ success: true }); - }) - .get("/deletion-impact/:userId", requireAuth, requireAdmin, getUserDeletionImpactDto, async (c) => { - const userId = c.req.param("userId"); - const impact = await authService.getUserDeletionImpact(userId); - return c.json(impact); - }) - .get("/org-members", requireAuth, requireOrgAdmin, getOrgMembersDto, async (c) => { + return c.json({ success: true }); + }, + ) + .get( + "/deletion-impact/:userId", + requireAuth, + requirePermission("instanceUsers.manage"), + getUserDeletionImpactDto, + async (c) => { + const userId = c.req.param("userId"); + const impact = await authService.getUserDeletionImpact(userId); + return c.json(impact); + }, + ) + .get("/org-members", requireAuth, requirePermission("organizationMembers.manage"), getOrgMembersDto, async (c) => { const organizationId = c.get("organizationId"); const result = await authService.getOrgMembers(organizationId); return c.json(result); @@ -74,7 +86,7 @@ export const authController = new Hono() .patch( "/org-members/:memberId/role", requireAuth, - requireOrgAdmin, + requirePermission("organizationMembers.manage"), updateMemberRoleDto, validator("json", updateMemberRoleBody), async (c) => { @@ -95,19 +107,25 @@ export const authController = new Hono() return c.json({ success: true }); }, ) - .delete("/org-members/:memberId", requireAuth, requireOrgAdmin, removeOrgMemberDto, async (c) => { - const memberId = c.req.param("memberId"); - const organizationId = c.get("organizationId"); + .delete( + "/org-members/:memberId", + requireAuth, + requirePermission("organizationMembers.manage"), + removeOrgMemberDto, + async (c) => { + const memberId = c.req.param("memberId"); + const organizationId = c.get("organizationId"); - const result = await authService.removeOrgMember(memberId, organizationId); + const result = await authService.removeOrgMember(memberId, organizationId); - if (!result.found) { - return c.json({ message: "Member not found" }, 404); - } + if (!result.found) { + return c.json({ message: "Member not found" }, 404); + } - if (result.isOwner) { - return c.json({ message: "Cannot remove the organization owner" }, 403); - } + if (result.isOwner) { + return c.json({ message: "Cannot remove the organization owner" }, 403); + } - return c.json({ success: true }); - }); + return c.json({ success: true }); + }, + ); diff --git a/app/server/modules/sso/__tests__/sso.registration.test.ts b/app/server/modules/sso/__tests__/sso.registration.test.ts index 1d55a76d..f41d43b8 100644 --- a/app/server/modules/sso/__tests__/sso.registration.test.ts +++ b/app/server/modules/sso/__tests__/sso.registration.test.ts @@ -1,10 +1,11 @@ -import { beforeEach, describe, expect, test } from "vitest"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; import { eq } from "drizzle-orm"; import { createApp } from "~/server/app"; import { db } from "~/server/db/db"; import { account, invitation, member, organization, ssoProvider, usersTable, verification } from "~/server/db/schema"; import { createTestSession, createTestSessionWithOrgAdmin } from "~/test/helpers/auth"; import { SSO_INVITATION_INTENT_COOKIE, ssoService } from "../sso.service"; +import { config } from "~/server/core/config"; const app = createApp(); const ssoRegisterUrl = new URL("/api/auth/sso/register", "http://localhost:3000").toString(); @@ -34,6 +35,10 @@ function buildRegisterBody(organizationId: string, suffix: string) { } describe("SSO provider registration authorization", () => { + afterEach(() => { + config.runtime = "server"; + }); + beforeEach(async () => { await db.delete(member); await db.delete(account); @@ -77,6 +82,25 @@ describe("SSO provider registration authorization", () => { expect(body.message).toBe("Only organization owners can register SSO providers"); }); + test("rejects provider registration when SSO management is unavailable", async () => { + config.runtime = "desktop"; + const { headers, organizationId } = await createTestSession(); + + const response = await app.request(ssoRegisterUrl, { + method: "POST", + headers: { + ...headers, + "Content-Type": "application/json", + }, + body: JSON.stringify(buildRegisterBody(organizationId, "desktop")), + }); + + expect(response.status).toBe(403); + + const body = await response.json(); + expect(body.message).toBe("Not available in desktop mode"); + }); + test("rejects users who are owners elsewhere but only members of the target organization", async () => { const { headers, user } = await createTestSession(); const targetOrgId = Bun.randomUUIDv7(); diff --git a/app/server/modules/sso/middlewares/authorize-registration.ts b/app/server/modules/sso/middlewares/authorize-registration.ts index 951acfbe..f07dcd39 100644 --- a/app/server/modules/sso/middlewares/authorize-registration.ts +++ b/app/server/modules/sso/middlewares/authorize-registration.ts @@ -1,6 +1,7 @@ import { APIError } from "better-auth/api"; import type { AuthMiddlewareContext } from "~/server/lib/auth"; import { db } from "~/server/db/db"; +import { checkPermissionForContext } from "~/server/lib/permission-service"; export const authorizeSsoRegistration = async (ctx: AuthMiddlewareContext) => { if (ctx.path !== "/sso/register") { @@ -39,7 +40,26 @@ export const authorizeSsoRegistration = async (ctx: AuthMiddlewareContext) => { throw new APIError("FORBIDDEN", { message: "You are not a member of this organization" }); } - if (membership.role !== "owner") { + const permission = checkPermissionForContext("ssoProvider.create", { + orgRole: membership.role, + authSource: "browser-session", + }); + + if (permission.allowed) { + return; + } + + if (permission.reason === "runtime") { + throw new APIError("FORBIDDEN", { message: "Not available in desktop mode" }); + } + + if (permission.reason === "authSource") { + throw new APIError("UNAUTHORIZED", { message: "Browser session required" }); + } + + if (permission.reason === "orgRole") { throw new APIError("FORBIDDEN", { message: "Only organization owners can register SSO providers" }); } + + throw new APIError("FORBIDDEN", { message: "Forbidden" }); }; diff --git a/app/server/modules/sso/sso.controller.ts b/app/server/modules/sso/sso.controller.ts index d0800058..91d0badc 100644 --- a/app/server/modules/sso/sso.controller.ts +++ b/app/server/modules/sso/sso.controller.ts @@ -15,19 +15,24 @@ import { updateSsoProviderAutoLinkingDto, } from "./sso.dto"; import { SSO_INVITATION_INTENT_COOKIE, ssoService } from "./sso.service"; -import { requireAuth, requireBrowserSession, requireOrgAdmin } from "../auth/auth.middleware"; +import { requireAuth, requireBrowserSession, requirePermission } from "../auth/auth.middleware"; import { auth } from "~/server/lib/auth"; import { mapAuthErrorToCode } from "./sso.errors"; import { config } from "~/server/core/config"; import { setCookie } from "hono/cookie"; import { normalizeEmail } from "./utils/sso-context"; +import { serverHasRuntimeFeature } from "~/server/lib/permission-service"; export const ssoController = new Hono() .get("/sso-providers", getPublicSsoProvidersDto, async (c) => { + if (!serverHasRuntimeFeature("ssoManagement")) { + return c.json({ providers: [] }); + } + const providers = await ssoService.getPublicSsoProviders(); return c.json(providers); }) - .get("/sso-settings", requireAuth, requireBrowserSession, requireOrgAdmin, getSsoSettingsDto, async (c) => { + .get("/sso-settings", requireAuth, requirePermission("sso.manage"), getSsoSettingsDto, async (c) => { const headers = c.req.raw.headers; const activeOrganizationId = c.get("organizationId"); @@ -110,8 +115,7 @@ export const ssoController = new Hono() .delete( "/sso-providers/:providerId", requireAuth, - requireBrowserSession, - requireOrgAdmin, + requirePermission("sso.manage"), deleteSsoProviderDto, async (c) => { const providerId = c.req.param("providerId"); @@ -129,8 +133,7 @@ export const ssoController = new Hono() .patch( "/sso-providers/:providerId/auto-linking", requireAuth, - requireBrowserSession, - requireOrgAdmin, + requirePermission("sso.manage"), updateSsoProviderAutoLinkingDto, validator("json", updateSsoProviderAutoLinkingBody), async (c) => { @@ -150,8 +153,7 @@ export const ssoController = new Hono() .delete( "/sso-invitations/:invitationId", requireAuth, - requireBrowserSession, - requireOrgAdmin, + requirePermission("sso.manage"), deleteSsoInvitationDto, async (c) => { const invitationId = c.req.param("invitationId"); diff --git a/app/server/modules/system/system.controller.ts b/app/server/modules/system/system.controller.ts index 6f685c89..e94de65b 100644 --- a/app/server/modules/system/system.controller.ts +++ b/app/server/modules/system/system.controller.ts @@ -15,7 +15,7 @@ import { type DevPanelDto, } from "./system.dto"; import { systemService } from "./system.service"; -import { requireAdmin, requireAuth, requireBrowserSession, requireOrgAdmin } from "../auth/auth.middleware"; +import { requireAuth, requirePermission } from "../auth/auth.middleware"; import { db } from "../../db/db"; import { usersTable } from "../../db/schema"; import { eq } from "drizzle-orm"; @@ -43,7 +43,7 @@ export const systemController = new Hono() }) .put( "/registration-status", - requireAdmin, + requirePermission("registration.manage"), setRegistrationStatusDto, validator("json", registrationStatusBody), async (c) => { @@ -56,8 +56,7 @@ export const systemController = new Hono() ) .post( "/restic-password", - requireBrowserSession, - requireOrgAdmin, + requirePermission("recoveryKey.download"), downloadResticPasswordDto, validator("json", downloadResticPasswordBodySchema), async (c) => {