refactor(auth): enforce named permissions on server routes (#984)

This commit is contained in:
Nico
2026-06-14 20:16:34 +02:00
committed by GitHub
parent 551e55aa53
commit 64e3bf48b9
6 changed files with 112 additions and 49 deletions

View File

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

View File

@@ -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<GetStatusDto>({ 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<UserDeletionImpactDto>(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<UserDeletionImpactDto>(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<OrgMembersDto>(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 });
},
);

View File

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

View File

@@ -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" });
};

View File

@@ -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<PublicSsoProvidersDto>({ providers: [] });
}
const providers = await ssoService.getPublicSsoProviders();
return c.json<PublicSsoProvidersDto>(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");

View File

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