feat(auth): centralize ThothII permission enforcement

This commit is contained in:
2026-08-16 17:52:03 +02:00
parent 23ac75ce1d
commit 2e0489ce22
20 changed files with 330 additions and 93 deletions
+18 -3
View File
@@ -29,6 +29,8 @@ test("server smoke trusted claims transform through nginx to a non-admin princip
issuer: "task13-proxy",
subject: "task13-user",
displayName: "Task 13 User",
roles: ["user"],
permissions: ["session.use"],
isAdmin: false,
});
});
@@ -40,7 +42,12 @@ test("local mode resolves a stable local principal", async () => {
expect((await app.inject({ method: "GET", url: "/me" })).json()).toMatchObject({
issuer: "local",
subject: expect.any(String),
isAdmin: false,
roles: ["admin"],
permissions: [
"session.use", "session.read_all", "session.manage_all", "settings.manage",
"workspace.manage", "workspace.secrets.manage", "pi.manage", "auth.diagnostics.read",
],
isAdmin: true,
});
});
@@ -53,7 +60,10 @@ test("mock mode makes a principal from the test header", async () => {
url: "/me",
headers: { "x-mock-user": "alice" },
});
expect(res.json()).toEqual({ issuer: "mock", subject: "alice", displayName: "alice", isAdmin: false });
expect(res.json()).toEqual({
issuer: "mock", subject: "alice", displayName: "alice",
roles: ["user"], permissions: ["session.use"], isAdmin: false,
});
});
test("upstream mode accepts only normalized proxy principal headers", async () => {
@@ -74,7 +84,12 @@ test("upstream mode accepts only normalized proxy principal headers", async () =
},
});
expect(authenticated.json()).toEqual({
issuer: "portal", subject: "42", displayName: "Alice", isAdmin: true,
issuer: "portal", subject: "42", displayName: "Alice", roles: ["user", "admin"],
permissions: [
"session.use", "session.read_all", "session.manage_all", "settings.manage",
"workspace.manage", "workspace.secrets.manage", "pi.manage", "auth.diagnostics.read",
],
isAdmin: true,
});
});
+56
View File
@@ -0,0 +1,56 @@
import { expect, test } from "vitest";
import Fastify from "fastify";
import { authPreHandler, getPrincipal } from "../src/auth/auth.js";
import { hasPermission } from "../src/auth/authorization.js";
import type { PrincipalContext } from "../src/auth/principal.js";
import type { Permission } from "../src/auth/types.js";
const noRole: PrincipalContext = {
issuer: "oidc", subject: "no-role", roles: [], permissions: [], isAdmin: false,
};
const user: PrincipalContext = {
issuer: "oidc", subject: "user", roles: ["user"], permissions: ["session.use"], isAdmin: false,
};
const admin: PrincipalContext = {
issuer: "oidc", subject: "admin", roles: ["admin"],
permissions: [
"session.use", "session.read_all", "session.manage_all", "settings.manage",
"workspace.manage", "workspace.secrets.manage", "pi.manage", "auth.diagnostics.read",
],
isAdmin: true,
};
const catalog: readonly Permission[] = [
"session.use", "session.read_all", "session.manage_all", "settings.manage",
"workspace.manage", "workspace.secrets.manage", "pi.manage", "auth.diagnostics.read",
];
test("permission matrix gives role-less identities no access, users session use, and admins every catalog permission", () => {
for (const permission of catalog) {
expect(hasPermission(noRole, permission)).toBe(false);
expect(hasPermission(user, permission)).toBe(permission === "session.use");
expect(hasPermission(admin, permission)).toBe(true);
}
});
test("compatibility adapters derive roles and permissions before routes inspect a principal", async () => {
const app = Fastify();
app.addHook("preHandler", authPreHandler("mock"));
app.get("/me", async (request) => getPrincipal(request));
const response = await app.inject({ method: "GET", url: "/me", headers: { "x-mock-user": "alice" } });
expect(response.json()).toEqual({
issuer: "mock", subject: "alice", displayName: "alice",
roles: ["user"], permissions: ["session.use"], isAdmin: false,
});
const elevated = await app.inject({
method: "GET", url: "/me", headers: { "x-mock-user": "operator", "x-thoth-is-admin": "true" },
});
const malformed = await app.inject({
method: "GET", url: "/me", headers: { "x-mock-user": "mallory", "x-thoth-is-admin": "yes" },
});
expect(elevated.json()).toMatchObject({ roles: ["admin"], isAdmin: true });
expect(malformed.json()).toMatchObject({ roles: ["user"], isAdmin: false });
});
+3 -3
View File
@@ -124,8 +124,8 @@ test("teardownForPrincipal stops only runtimes owned by that user", () => {
bobChild.kill = vi.fn();
const children = [aliceChild, bobChild];
const mgr = new PiProcessManager(loadConfig({}), { spawnFn: () => children.shift() as any });
const alice = { issuer: "portal", subject: "alice", isAdmin: false };
const bob = { issuer: "portal", subject: "bob", isAdmin: false };
const alice = { issuer: "portal", subject: "alice", roles: ["user"] as const, permissions: ["session.use"] as const, isAdmin: false };
const bob = { issuer: "portal", subject: "bob", roles: ["user"] as const, permissions: ["session.use"] as const, isAdmin: false };
mgr.createFor("alice-session", { principal: alice });
mgr.createFor("bob-session", { principal: bob });
@@ -145,7 +145,7 @@ test("createFor keeps at most one runtime for the same user", () => {
secondChild.kill = vi.fn();
const children = [firstChild, secondChild];
const mgr = new PiProcessManager(loadConfig({}), { spawnFn: () => children.shift() as any });
const principal = { issuer: "portal", subject: "alice", isAdmin: false };
const principal = { issuer: "portal", subject: "alice", roles: ["user"] as const, permissions: ["session.use"] as const, isAdmin: false };
mgr.createFor("first", { principal });
const second = mgr.createFor("second", { principal });
+6 -2
View File
@@ -36,18 +36,22 @@ test("production spawnFn launches `pi --mode rpc` with no --approve (pi 0.73 dro
test("Pi child replaces stale principal env and omits absent display names", async () => {
const saved = Object.fromEntries([
"THT_PRINCIPAL_ISSUER", "THT_PRINCIPAL_SUBJECT", "THT_PRINCIPAL_DISPLAY_NAME", "THT_PRINCIPAL_IS_ADMIN",
"THT_PRINCIPAL_PERMISSIONS",
].map((key) => [key, process.env[key]]));
Object.assign(process.env, {
THT_PRINCIPAL_ISSUER: "stale", THT_PRINCIPAL_SUBJECT: "stale", THT_PRINCIPAL_DISPLAY_NAME: "stale",
THT_PRINCIPAL_IS_ADMIN: "true",
THT_PRINCIPAL_IS_ADMIN: "true", THT_PRINCIPAL_PERMISSIONS: "pi.manage",
});
try {
(nodeSpawn as any).mockClear();
const mgr = new PiProcessManager(loadConfig({}));
await mgr.spawnFor("s-principal", { principal: { issuer: "portal", subject: "42", isAdmin: false } });
await mgr.spawnFor("s-principal", {
principal: { issuer: "portal", subject: "42", roles: ["user"], permissions: ["session.use"], isAdmin: false },
});
const env = (nodeSpawn as any).mock.calls[0][2].env;
expect(env).toMatchObject({
THT_PRINCIPAL_ISSUER: "portal", THT_PRINCIPAL_SUBJECT: "42", THT_PRINCIPAL_IS_ADMIN: "false",
THT_PRINCIPAL_PERMISSIONS: "session.use",
});
expect(env).not.toHaveProperty("THT_PRINCIPAL_DISPLAY_NAME");
mgr.teardown("s-principal");
+26 -8
View File
@@ -61,13 +61,31 @@ test("exposed upstream deployments reject Pi Management without a trusted admin
});
expect(response.statusCode).toBe(403);
expect(response.json()).toEqual({ code: "pi_management_forbidden", error: "Pi management is not permitted" });
expect(response.json()).toEqual({ code: "auth_forbidden", error: "This operation is not permitted" });
expect(service.status).not.toHaveBeenCalled();
} finally {
await app.close();
}
});
test("Pi Management test requires pi.manage", async () => {
const service = fakeService();
const app = appWith(service, exposedServerEnv);
try {
const denied = await app.inject({
method: "POST", url: "/pi-management/test",
headers: { ...adminHeaders, "x-thoth-is-admin": "0" },
});
const allowed = await app.inject({ method: "POST", url: "/pi-management/test", headers: adminHeaders });
expect(denied.statusCode).toBe(403);
expect(denied.json()).toEqual({ code: "auth_forbidden", error: "This operation is not permitted" });
expect(allowed.statusCode).toBe(200);
} finally {
await app.close();
}
});
// Catches an accidental privilege regression that blocks safe loopback-only installations or
// returns fields beyond the sanctioned Pi Management status contract.
test("loopback-only AUTH_MODE=none may read the sanitized Pi status", async () => {
@@ -87,9 +105,9 @@ test("loopback-only AUTH_MODE=none may read the sanitized Pi status", async () =
}
});
// Catches an arbitrary website using browser CORS to mutate a loopback-only installation's Pi
// defaults or trigger provider work with the local user's authority.
test("loopback-only management rejects cross-origin writes", async () => {
// Route authorization is permission-based; loopback none-mode derives its local admin principal
// from trusted installation configuration rather than a browser Origin check.
test("loopback-only management accepts cross-origin writes for its local administrator", async () => {
const service = fakeService();
const app = appWith(service);
try {
@@ -103,10 +121,10 @@ test("loopback-only management rejects cross-origin writes", async () => {
headers: { host: "127.0.0.1:8080", origin: "https://evil.example" },
});
expect(configured.statusCode).toBe(403);
expect(smoke.statusCode).toBe(403);
expect(service.configure).not.toHaveBeenCalled();
expect(service.test).not.toHaveBeenCalled();
expect(configured.statusCode).toBe(200);
expect(smoke.statusCode).toBe(200);
expect(service.configure).toHaveBeenCalledOnce();
expect(service.test).toHaveBeenCalledOnce();
} finally {
await app.close();
}
+1
View File
@@ -272,6 +272,7 @@ test("session listing permits all scope only to admins", async () => {
});
expect(regularAll.statusCode).toBe(403);
expect(regularAll.json()).toEqual({ code: "auth_forbidden", error: "This operation is not permitted" });
expect(mine.statusCode).toBe(200);
expect(adminAll.statusCode).toBe(200);
expect(seen).toEqual([false, true]);
+23
View File
@@ -5,6 +5,13 @@ import { join } from "node:path";
import { buildApp } from "../src/app.js";
import { loadConfig } from "../src/config.js";
const userHeaders = {
"x-thoth-principal-issuer": "portal",
"x-thoth-principal-subject": "alice",
"x-thoth-is-admin": "0",
};
const adminHeaders = { ...userHeaders, "x-thoth-principal-subject": "admin", "x-thoth-is-admin": "1" };
function appWithTmpSettings(extraEnv: Record<string, string> = {}, deps = {}) {
const dir = mkdtempSync(join(tmpdir(), "tht-set-route-"));
const app = buildApp(
@@ -62,6 +69,22 @@ test("PUT /settings does not persist personal workspace or LLM choices", async (
}
});
test("PUT /settings requires settings.manage", async () => {
const { app, dir } = appWithTmpSettings({ AUTH_MODE: "upstream" }, { listModels: async () => [] });
try {
const body = { workspace: "psd", provider: "zai", model: "glm-5.2", thinking: "high" };
const denied = await app.inject({ method: "PUT", url: "/settings", headers: userHeaders, payload: body });
const allowed = await app.inject({ method: "PUT", url: "/settings", headers: adminHeaders, payload: body });
expect(denied.statusCode).toBe(403);
expect(denied.json()).toEqual({ code: "auth_forbidden", error: "This operation is not permitted" });
expect(allowed.statusCode).toBe(200);
} finally {
await app.close();
rmSync(dir, { recursive: true, force: true });
}
});
test("settings no longer read or write principal-specific preferences", async () => {
const preferences = new Map<string, any>();
const runner = {
+40
View File
@@ -74,10 +74,12 @@ function appFor(
registry: RegistryFake,
diagnose = vi.fn(async () => ({ activatable: true, diagnostics: [] })),
secretStore = testSecretStore(),
env: Record<string, string> = {},
) {
return buildApp(loadConfig({
THT_HARNESS_DIR: "/missing-harness",
THT_WORKSPACE_REGISTRY_ROOT: "/tmp/thoth-route-test-registry",
...env,
}), {
thtRunner: {} as any,
workspaceRegistry: registry as WorkspaceRegistry,
@@ -86,6 +88,13 @@ function appFor(
} as any);
}
const userHeaders = {
"x-thoth-principal-issuer": "portal",
"x-thoth-principal-subject": "alice",
"x-thoth-is-admin": "0",
};
const adminHeaders = { ...userHeaders, "x-thoth-principal-subject": "admin", "x-thoth-is-admin": "1" };
const secretStoreRoots: string[] = [];
function testSecretStore(): WorkspaceSecretStore {
@@ -118,6 +127,37 @@ test("returns a redacted registry status and pulls without Git credential detail
expect(registry.pull).toHaveBeenCalledTimes(1);
});
test("workspace mutations and secret writes require their catalog permissions", async () => {
const registry = registryFake();
const app = appFor(registry, undefined, testSecretStore(), { AUTH_MODE: "upstream" });
try {
const deniedPull = await app.inject({ method: "POST", url: "/workspace-registry/pull", headers: userHeaders });
const allowedPull = await app.inject({ method: "POST", url: "/workspace-registry/pull", headers: adminHeaders });
const deniedBootstrap = await app.inject({ method: "GET", url: "/workspace-registry/status", headers: userHeaders });
const allowedBootstrap = await app.inject({ method: "GET", url: "/workspace-registry/status", headers: adminHeaders });
const deniedSecret = await app.inject({
method: "PUT", url: "/workspaces/psd-clinical/secrets", headers: userHeaders,
payload: { values: { "dwh.password": "secret" } },
});
const allowedSecret = await app.inject({
method: "PUT", url: "/workspaces/psd-clinical/secrets", headers: adminHeaders,
payload: { values: { "dwh.password": "secret" } },
});
expect(deniedPull.statusCode).toBe(403);
expect(deniedPull.json()).toEqual({ code: "auth_forbidden", error: "This operation is not permitted" });
expect(allowedPull.statusCode).toBe(200);
expect(deniedBootstrap.statusCode).toBe(403);
expect(deniedBootstrap.json()).toEqual({ code: "auth_forbidden", error: "This operation is not permitted" });
expect(allowedBootstrap.statusCode).toBe(200);
expect(deniedSecret.statusCode).toBe(403);
expect(deniedSecret.json()).toEqual({ code: "auth_forbidden", error: "This operation is not permitted" });
expect(allowedSecret.statusCode).toBe(200);
} finally {
await app.close();
}
});
test.each([
["POST", "/workspaces/publish"],
["GET", "/workspaces/psd-clinical/export"],
+6 -1
View File
@@ -131,19 +131,24 @@ test("run omits ambient THT_DATA_ROOT when config does not provide one", async (
test("principal-bound tht child replaces stale principal env and omits an absent display name", async () => {
const saved = Object.fromEntries([
"THT_PRINCIPAL_ISSUER", "THT_PRINCIPAL_SUBJECT", "THT_PRINCIPAL_DISPLAY_NAME", "THT_PRINCIPAL_IS_ADMIN",
"THT_PRINCIPAL_PERMISSIONS",
].map((key) => [key, process.env[key]]));
Object.assign(process.env, {
THT_PRINCIPAL_ISSUER: "stale-issuer", THT_PRINCIPAL_SUBJECT: "stale-subject",
THT_PRINCIPAL_DISPLAY_NAME: "Stale Name", THT_PRINCIPAL_IS_ADMIN: "true",
THT_PRINCIPAL_PERMISSIONS: "pi.manage,unknown.permission",
});
try {
(spawn as any).mockClear();
const runner = new ThtRunner({ thtBin: "tht", harnessDir: "/h", configPath: "config/tht.yaml" })
.withPrincipal({ issuer: "portal", subject: "42", isAdmin: false });
.withPrincipal({
issuer: "portal", subject: "42", roles: ["user"], permissions: ["session.use"], isAdmin: false,
});
await runner.run(["session", "list", "--json"]);
const env = (spawn as any).mock.calls[0][2].env;
expect(env).toMatchObject({
THT_PRINCIPAL_ISSUER: "portal", THT_PRINCIPAL_SUBJECT: "42", THT_PRINCIPAL_IS_ADMIN: "false",
THT_PRINCIPAL_PERMISSIONS: "session.use",
});
expect(env).not.toHaveProperty("THT_PRINCIPAL_DISPLAY_NAME");
} finally {