mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
feat: add permission checks and ownership scoping to user-files routes
Replace getAuthUser (optional auth) with requirePermission("files:own") on all
user-files routes, enforcing mandatory authentication and ownership checks.
Admin users with files:all permission bypass ownership restrictions. Returns 404
(not 403) for ownership failures to avoid leaking resource existence.
This commit is contained in:
@@ -12,6 +12,7 @@
|
|||||||
import { randomUUID } from "node:crypto";
|
import { randomUUID } from "node:crypto";
|
||||||
import { createReadStream } from "node:fs";
|
import { createReadStream } from "node:fs";
|
||||||
import { extname } from "node:path";
|
import { extname } from "node:path";
|
||||||
|
import type { Role } from "@stirling-image/shared";
|
||||||
import { and, desc, eq, like, sql } from "drizzle-orm";
|
import { and, desc, eq, like, sql } from "drizzle-orm";
|
||||||
import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
|
import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
|
||||||
import sharp from "sharp";
|
import sharp from "sharp";
|
||||||
@@ -27,7 +28,7 @@ import {
|
|||||||
} from "../lib/file-storage.js";
|
} from "../lib/file-storage.js";
|
||||||
import { validateImageBuffer } from "../lib/file-validation.js";
|
import { validateImageBuffer } from "../lib/file-validation.js";
|
||||||
import { sanitizeFilename } from "../lib/filename.js";
|
import { sanitizeFilename } from "../lib/filename.js";
|
||||||
import { getAuthUser } from "../plugins/auth.js";
|
import { hasPermission, requirePermission } from "../permissions.js";
|
||||||
|
|
||||||
// ── Helpers ────────────────────────────────────────────────────────
|
// ── Helpers ────────────────────────────────────────────────────────
|
||||||
|
|
||||||
@@ -97,8 +98,9 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
}>,
|
}>,
|
||||||
reply: FastifyReply,
|
reply: FastifyReply,
|
||||||
) => {
|
) => {
|
||||||
const user = getAuthUser(request);
|
const user = requirePermission("files:own")(request, reply);
|
||||||
const userId = user?.id ?? null;
|
if (!user) return;
|
||||||
|
const canSeeAll = hasPermission(user.role as Role, "files:all");
|
||||||
|
|
||||||
const limit = Math.min(parseInt(request.query.limit ?? "50", 10) || 50, 200);
|
const limit = Math.min(parseInt(request.query.limit ?? "50", 10) || 50, 200);
|
||||||
const offset = parseInt(request.query.offset ?? "0", 10) || 0;
|
const offset = parseInt(request.query.offset ?? "0", 10) || 0;
|
||||||
@@ -113,8 +115,8 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
// Build the where clauses
|
// Build the where clauses
|
||||||
const conditions = [latestCondition];
|
const conditions = [latestCondition];
|
||||||
|
|
||||||
if (userId) {
|
if (!canSeeAll) {
|
||||||
conditions.push(eq(schema.userFiles.userId, userId));
|
conditions.push(eq(schema.userFiles.userId, user.id));
|
||||||
}
|
}
|
||||||
|
|
||||||
if (search) {
|
if (search) {
|
||||||
@@ -153,8 +155,9 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
* Validates each (magic bytes + dimensions), stores to disk, creates DB record.
|
* Validates each (magic bytes + dimensions), stores to disk, creates DB record.
|
||||||
*/
|
*/
|
||||||
app.post("/api/v1/files/upload", async (request: FastifyRequest, reply: FastifyReply) => {
|
app.post("/api/v1/files/upload", async (request: FastifyRequest, reply: FastifyReply) => {
|
||||||
const user = getAuthUser(request);
|
const user = requirePermission("files:own")(request, reply);
|
||||||
const userId = user?.id ?? null;
|
if (!user) return;
|
||||||
|
const userId = user.id;
|
||||||
|
|
||||||
const created: ReturnType<typeof serializeFile>[] = [];
|
const created: ReturnType<typeof serializeFile>[] = [];
|
||||||
|
|
||||||
@@ -231,6 +234,10 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
app.get(
|
app.get(
|
||||||
"/api/v1/files/:id",
|
"/api/v1/files/:id",
|
||||||
async (request: FastifyRequest<{ Params: { id: string } }>, reply: FastifyReply) => {
|
async (request: FastifyRequest<{ Params: { id: string } }>, reply: FastifyReply) => {
|
||||||
|
const user = requirePermission("files:own")(request, reply);
|
||||||
|
if (!user) return;
|
||||||
|
const canSeeAll = hasPermission(user.role as Role, "files:all");
|
||||||
|
|
||||||
const { id } = request.params;
|
const { id } = request.params;
|
||||||
|
|
||||||
const file = db.select().from(schema.userFiles).where(eq(schema.userFiles.id, id)).get();
|
const file = db.select().from(schema.userFiles).where(eq(schema.userFiles.id, id)).get();
|
||||||
@@ -239,6 +246,10 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
return reply.status(404).send({ error: "File not found" });
|
return reply.status(404).send({ error: "File not found" });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!canSeeAll && file.userId !== user.id) {
|
||||||
|
return reply.status(404).send({ error: "File not found" });
|
||||||
|
}
|
||||||
|
|
||||||
// Walk the full version chain using a recursive CTE.
|
// Walk the full version chain using a recursive CTE.
|
||||||
// First find the root ancestor, then collect all descendants.
|
// First find the root ancestor, then collect all descendants.
|
||||||
interface ChainRow {
|
interface ChainRow {
|
||||||
@@ -308,6 +319,10 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
app.get(
|
app.get(
|
||||||
"/api/v1/files/:id/download",
|
"/api/v1/files/:id/download",
|
||||||
async (request: FastifyRequest<{ Params: { id: string } }>, reply: FastifyReply) => {
|
async (request: FastifyRequest<{ Params: { id: string } }>, reply: FastifyReply) => {
|
||||||
|
const user = requirePermission("files:own")(request, reply);
|
||||||
|
if (!user) return;
|
||||||
|
const canSeeAll = hasPermission(user.role as Role, "files:all");
|
||||||
|
|
||||||
const { id } = request.params;
|
const { id } = request.params;
|
||||||
|
|
||||||
const file = db.select().from(schema.userFiles).where(eq(schema.userFiles.id, id)).get();
|
const file = db.select().from(schema.userFiles).where(eq(schema.userFiles.id, id)).get();
|
||||||
@@ -316,6 +331,10 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
return reply.status(404).send({ error: "File not found" });
|
return reply.status(404).send({ error: "File not found" });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!canSeeAll && file.userId !== user.id) {
|
||||||
|
return reply.status(404).send({ error: "File not found" });
|
||||||
|
}
|
||||||
|
|
||||||
const filePath = getStoredFilePath(file.storedName);
|
const filePath = getStoredFilePath(file.storedName);
|
||||||
|
|
||||||
const stream = createReadStream(filePath);
|
const stream = createReadStream(filePath);
|
||||||
@@ -341,6 +360,10 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
app.get(
|
app.get(
|
||||||
"/api/v1/files/:id/thumbnail",
|
"/api/v1/files/:id/thumbnail",
|
||||||
async (request: FastifyRequest<{ Params: { id: string } }>, reply: FastifyReply) => {
|
async (request: FastifyRequest<{ Params: { id: string } }>, reply: FastifyReply) => {
|
||||||
|
const user = requirePermission("files:own")(request, reply);
|
||||||
|
if (!user) return;
|
||||||
|
const canSeeAll = hasPermission(user.role as Role, "files:all");
|
||||||
|
|
||||||
const { id } = request.params;
|
const { id } = request.params;
|
||||||
|
|
||||||
const file = db.select().from(schema.userFiles).where(eq(schema.userFiles.id, id)).get();
|
const file = db.select().from(schema.userFiles).where(eq(schema.userFiles.id, id)).get();
|
||||||
@@ -349,6 +372,10 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
return reply.status(404).send({ error: "File not found" });
|
return reply.status(404).send({ error: "File not found" });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!canSeeAll && file.userId !== user.id) {
|
||||||
|
return reply.status(404).send({ error: "File not found" });
|
||||||
|
}
|
||||||
|
|
||||||
// Serve from disk cache if available
|
// Serve from disk cache if available
|
||||||
const cached = await getCachedThumbnail(file.storedName);
|
const cached = await getCachedThumbnail(file.storedName);
|
||||||
if (cached) {
|
if (cached) {
|
||||||
@@ -386,6 +413,10 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
* For each id, deletes the entire version chain (all ancestors and descendants).
|
* For each id, deletes the entire version chain (all ancestors and descendants).
|
||||||
*/
|
*/
|
||||||
app.delete("/api/v1/files", async (request: FastifyRequest, reply: FastifyReply) => {
|
app.delete("/api/v1/files", async (request: FastifyRequest, reply: FastifyReply) => {
|
||||||
|
const user = requirePermission("files:own")(request, reply);
|
||||||
|
if (!user) return;
|
||||||
|
const canDeleteAll = hasPermission(user.role as Role, "files:all");
|
||||||
|
|
||||||
const body = request.body as { ids?: unknown } | null;
|
const body = request.body as { ids?: unknown } | null;
|
||||||
|
|
||||||
if (!Array.isArray(body?.ids) || body.ids.length === 0) {
|
if (!Array.isArray(body?.ids) || body.ids.length === 0) {
|
||||||
@@ -402,14 +433,15 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
interface DeleteChainRow {
|
interface DeleteChainRow {
|
||||||
id: string;
|
id: string;
|
||||||
stored_name: string;
|
stored_name: string;
|
||||||
|
user_id: string | null;
|
||||||
}
|
}
|
||||||
|
|
||||||
for (const id of ids) {
|
for (const id of ids) {
|
||||||
// Collect all files in the chain using a recursive CTE
|
// Collect all files in the chain using a recursive CTE
|
||||||
const chainRows = sqlite
|
const chainRows = sqlite
|
||||||
.prepare(`
|
.prepare(`
|
||||||
WITH RECURSIVE chain(id, stored_name) AS (
|
WITH RECURSIVE chain(id, stored_name, user_id) AS (
|
||||||
SELECT f.id, f.stored_name
|
SELECT f.id, f.stored_name, f.user_id
|
||||||
FROM user_files f
|
FROM user_files f
|
||||||
WHERE f.id = (
|
WHERE f.id = (
|
||||||
WITH RECURSIVE ancestors(id, parent_id) AS (
|
WITH RECURSIVE ancestors(id, parent_id) AS (
|
||||||
@@ -421,14 +453,19 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
SELECT id FROM ancestors WHERE parent_id IS NULL LIMIT 1
|
SELECT id FROM ancestors WHERE parent_id IS NULL LIMIT 1
|
||||||
)
|
)
|
||||||
UNION ALL
|
UNION ALL
|
||||||
SELECT child.id, child.stored_name
|
SELECT child.id, child.stored_name, child.user_id
|
||||||
FROM user_files child
|
FROM user_files child
|
||||||
INNER JOIN chain c ON child.parent_id = c.id
|
INNER JOIN chain c ON child.parent_id = c.id
|
||||||
)
|
)
|
||||||
SELECT id, stored_name FROM chain
|
SELECT id, stored_name, user_id FROM chain
|
||||||
`)
|
`)
|
||||||
.all(id) as DeleteChainRow[];
|
.all(id) as DeleteChainRow[];
|
||||||
|
|
||||||
|
// Check ownership of the root file (first in chain)
|
||||||
|
if (chainRows.length > 0 && !canDeleteAll && chainRows[0].user_id !== user.id) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
for (const row of chainRows) {
|
for (const row of chainRows) {
|
||||||
await deleteStoredFile(row.stored_name);
|
await deleteStoredFile(row.stored_name);
|
||||||
await deleteThumbnail(row.stored_name);
|
await deleteThumbnail(row.stored_name);
|
||||||
@@ -437,8 +474,7 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const user = getAuthUser(request);
|
auditLog(request.log, "FILE_DELETED", { userId: user.id, count: deletedCount, ids });
|
||||||
auditLog(request.log, "FILE_DELETED", { userId: user?.id, count: deletedCount, ids });
|
|
||||||
|
|
||||||
return reply.send({ deleted: deletedCount });
|
return reply.send({ deleted: deletedCount });
|
||||||
});
|
});
|
||||||
@@ -453,8 +489,10 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
* toolId — the tool that produced this result
|
* toolId — the tool that produced this result
|
||||||
*/
|
*/
|
||||||
app.post("/api/v1/files/save-result", async (request: FastifyRequest, reply: FastifyReply) => {
|
app.post("/api/v1/files/save-result", async (request: FastifyRequest, reply: FastifyReply) => {
|
||||||
const user = getAuthUser(request);
|
const user = requirePermission("files:own")(request, reply);
|
||||||
const userId = user?.id ?? null;
|
if (!user) return;
|
||||||
|
const canSeeAll = hasPermission(user.role as Role, "files:all");
|
||||||
|
const userId = user.id;
|
||||||
|
|
||||||
let fileBuffer: Buffer | null = null;
|
let fileBuffer: Buffer | null = null;
|
||||||
let filename = "result";
|
let filename = "result";
|
||||||
@@ -504,6 +542,10 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
return reply.status(404).send({ error: "Parent file not found" });
|
return reply.status(404).send({ error: "Parent file not found" });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!canSeeAll && parent.userId !== user.id) {
|
||||||
|
return reply.status(404).send({ error: "Parent file not found" });
|
||||||
|
}
|
||||||
|
|
||||||
const nextVersion = parent.version + 1;
|
const nextVersion = parent.version + 1;
|
||||||
|
|
||||||
// Build the tool chain: append the new toolId to the parent's chain
|
// Build the tool chain: append the new toolId to the parent's chain
|
||||||
|
|||||||
@@ -403,3 +403,114 @@ describe("API key ownership scoping", () => {
|
|||||||
expect(res.statusCode).toBe(404);
|
expect(res.statusCode).toBe(404);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("file ownership scoping", () => {
|
||||||
|
let userAToken: string;
|
||||||
|
let userBToken: string;
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
// Create two users (fileuserA and fileuserB)
|
||||||
|
for (const name of ["fileuserA", "fileuserB"]) {
|
||||||
|
await testApp.app.inject({
|
||||||
|
method: "POST",
|
||||||
|
url: "/api/auth/register",
|
||||||
|
headers: { authorization: `Bearer ${adminToken}` },
|
||||||
|
payload: { username: name, password: "TestPass1", role: "user" },
|
||||||
|
});
|
||||||
|
db.update(schema.users)
|
||||||
|
.set({ mustChangePassword: false })
|
||||||
|
.where(eq(schema.users.username, name))
|
||||||
|
.run();
|
||||||
|
}
|
||||||
|
const loginA = await testApp.app.inject({
|
||||||
|
method: "POST",
|
||||||
|
url: "/api/auth/login",
|
||||||
|
payload: { username: "fileuserA", password: "TestPass1" },
|
||||||
|
});
|
||||||
|
userAToken = JSON.parse(loginA.body).token;
|
||||||
|
const loginB = await testApp.app.inject({
|
||||||
|
method: "POST",
|
||||||
|
url: "/api/auth/login",
|
||||||
|
payload: { username: "fileuserB", password: "TestPass1" },
|
||||||
|
});
|
||||||
|
userBToken = JSON.parse(loginB.body).token;
|
||||||
|
});
|
||||||
|
|
||||||
|
it("unauthenticated request to files returns 401", async () => {
|
||||||
|
const res = await testApp.app.inject({ method: "GET", url: "/api/v1/files" });
|
||||||
|
expect(res.statusCode).toBe(401);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("user A cannot access user B's file by ID", async () => {
|
||||||
|
// Upload as user A
|
||||||
|
const testImage = readFileSync(join(import.meta.dirname, "..", "fixtures", "test-200x150.png"));
|
||||||
|
const { body: uploadBody, contentType } = createMultipartPayload([
|
||||||
|
{ name: "file", filename: "test.png", contentType: "image/png", content: testImage },
|
||||||
|
]);
|
||||||
|
const uploadRes = await testApp.app.inject({
|
||||||
|
method: "POST",
|
||||||
|
url: "/api/v1/files/upload",
|
||||||
|
headers: { "content-type": contentType, authorization: `Bearer ${userAToken}` },
|
||||||
|
body: uploadBody,
|
||||||
|
});
|
||||||
|
const fileId = JSON.parse(uploadRes.body).files[0].id;
|
||||||
|
|
||||||
|
// User B tries to access it - should get 404
|
||||||
|
const res = await testApp.app.inject({
|
||||||
|
method: "GET",
|
||||||
|
url: `/api/v1/files/${fileId}`,
|
||||||
|
headers: { authorization: `Bearer ${userBToken}` },
|
||||||
|
});
|
||||||
|
expect(res.statusCode).toBe(404);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("user B cannot download user A's file", async () => {
|
||||||
|
// List user A's files
|
||||||
|
const listRes = await testApp.app.inject({
|
||||||
|
method: "GET",
|
||||||
|
url: "/api/v1/files",
|
||||||
|
headers: { authorization: `Bearer ${userAToken}` },
|
||||||
|
});
|
||||||
|
const files = JSON.parse(listRes.body).files;
|
||||||
|
expect(files.length).toBeGreaterThan(0);
|
||||||
|
|
||||||
|
// User B tries to download
|
||||||
|
const res = await testApp.app.inject({
|
||||||
|
method: "GET",
|
||||||
|
url: `/api/v1/files/${files[0].id}/download`,
|
||||||
|
headers: { authorization: `Bearer ${userBToken}` },
|
||||||
|
});
|
||||||
|
expect(res.statusCode).toBe(404);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("admin can access any user's file", async () => {
|
||||||
|
const listRes = await testApp.app.inject({
|
||||||
|
method: "GET",
|
||||||
|
url: "/api/v1/files",
|
||||||
|
headers: { authorization: `Bearer ${adminToken}` },
|
||||||
|
});
|
||||||
|
expect(listRes.statusCode).toBe(200);
|
||||||
|
// Admin should see files from user A
|
||||||
|
const files = JSON.parse(listRes.body).files;
|
||||||
|
expect(files.length).toBeGreaterThan(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("user B cannot delete user A's file", async () => {
|
||||||
|
const listRes = await testApp.app.inject({
|
||||||
|
method: "GET",
|
||||||
|
url: "/api/v1/files",
|
||||||
|
headers: { authorization: `Bearer ${userAToken}` },
|
||||||
|
});
|
||||||
|
const files = JSON.parse(listRes.body).files;
|
||||||
|
expect(files.length).toBeGreaterThan(0);
|
||||||
|
|
||||||
|
const res = await testApp.app.inject({
|
||||||
|
method: "DELETE",
|
||||||
|
url: "/api/v1/files",
|
||||||
|
headers: { authorization: `Bearer ${userBToken}` },
|
||||||
|
payload: { ids: [files[0].id] },
|
||||||
|
});
|
||||||
|
const body = JSON.parse(res.body);
|
||||||
|
expect(body.deleted).toBe(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -42,6 +42,7 @@ import { registerProgressRoutes } from "../../apps/api/src/routes/progress.js";
|
|||||||
import { settingsRoutes } from "../../apps/api/src/routes/settings.js";
|
import { settingsRoutes } from "../../apps/api/src/routes/settings.js";
|
||||||
import { teamsRoutes } from "../../apps/api/src/routes/teams.js";
|
import { teamsRoutes } from "../../apps/api/src/routes/teams.js";
|
||||||
import { registerToolRoutes } from "../../apps/api/src/routes/tools/index.js";
|
import { registerToolRoutes } from "../../apps/api/src/routes/tools/index.js";
|
||||||
|
import { userFileRoutes } from "../../apps/api/src/routes/user-files.js";
|
||||||
|
|
||||||
// Run migrations to create all tables in the temp DB
|
// Run migrations to create all tables in the temp DB
|
||||||
runMigrations();
|
runMigrations();
|
||||||
@@ -84,6 +85,9 @@ export async function buildTestApp(): Promise<TestApp> {
|
|||||||
// File upload/download routes
|
// File upload/download routes
|
||||||
await fileRoutes(app);
|
await fileRoutes(app);
|
||||||
|
|
||||||
|
// User file library routes
|
||||||
|
await userFileRoutes(app);
|
||||||
|
|
||||||
// Tool routes
|
// Tool routes
|
||||||
await registerToolRoutes(app);
|
await registerToolRoutes(app);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user