mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
feat: add permission checks and admin override to API key routes
Replace requireAuth with requirePermission(apikeys:own). Admin users with apikeys:all can see and delete any user's keys.
This commit is contained in:
@@ -6,16 +6,18 @@
|
||||
* DELETE /api/v1/api-keys/:id — Delete an API key
|
||||
*/
|
||||
import { randomBytes, randomUUID } from "node:crypto";
|
||||
import type { Role } from "@stirling-image/shared";
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
|
||||
import { db, schema } from "../db/index.js";
|
||||
import { auditLog } from "../lib/audit.js";
|
||||
import { computeKeyPrefix, hashPassword, requireAuth } from "../plugins/auth.js";
|
||||
import { hasPermission, requirePermission } from "../permissions.js";
|
||||
import { computeKeyPrefix, hashPassword } from "../plugins/auth.js";
|
||||
|
||||
export async function apiKeyRoutes(app: FastifyInstance): Promise<void> {
|
||||
// POST /api/v1/api-keys — Generate a new API key
|
||||
app.post("/api/v1/api-keys", async (request: FastifyRequest, reply: FastifyReply) => {
|
||||
const user = requireAuth(request, reply);
|
||||
const user = requirePermission("apikeys:own")(request, reply);
|
||||
if (!user) return;
|
||||
|
||||
const body = request.body as { name?: string } | null;
|
||||
@@ -57,19 +59,21 @@ export async function apiKeyRoutes(app: FastifyInstance): Promise<void> {
|
||||
|
||||
// GET /api/v1/api-keys — List user's API keys (never returns the key itself)
|
||||
app.get("/api/v1/api-keys", async (request: FastifyRequest, reply: FastifyReply) => {
|
||||
const user = requireAuth(request, reply);
|
||||
const user = requirePermission("apikeys:own")(request, reply);
|
||||
if (!user) return;
|
||||
|
||||
const keys = db
|
||||
const canSeeAll = hasPermission(user.role as Role, "apikeys:all");
|
||||
|
||||
const query = db
|
||||
.select({
|
||||
id: schema.apiKeys.id,
|
||||
name: schema.apiKeys.name,
|
||||
createdAt: schema.apiKeys.createdAt,
|
||||
lastUsedAt: schema.apiKeys.lastUsedAt,
|
||||
})
|
||||
.from(schema.apiKeys)
|
||||
.where(eq(schema.apiKeys.userId, user.id))
|
||||
.all();
|
||||
.from(schema.apiKeys);
|
||||
|
||||
const keys = canSeeAll ? query.all() : query.where(eq(schema.apiKeys.userId, user.id)).all();
|
||||
|
||||
return reply.send({
|
||||
apiKeys: keys.map((k) => ({
|
||||
@@ -85,16 +89,21 @@ export async function apiKeyRoutes(app: FastifyInstance): Promise<void> {
|
||||
app.delete(
|
||||
"/api/v1/api-keys/:id",
|
||||
async (request: FastifyRequest<{ Params: { id: string } }>, reply: FastifyReply) => {
|
||||
const user = requireAuth(request, reply);
|
||||
const user = requirePermission("apikeys:own")(request, reply);
|
||||
if (!user) return;
|
||||
|
||||
const { id } = request.params;
|
||||
|
||||
// Ensure the key belongs to the requesting user
|
||||
// Admin can delete any key; regular users can only delete their own
|
||||
const canDeleteAll = hasPermission(user.role as Role, "apikeys:all");
|
||||
const existing = db
|
||||
.select()
|
||||
.from(schema.apiKeys)
|
||||
.where(and(eq(schema.apiKeys.id, id), eq(schema.apiKeys.userId, user.id)))
|
||||
.where(
|
||||
canDeleteAll
|
||||
? eq(schema.apiKeys.id, id)
|
||||
: and(eq(schema.apiKeys.id, id), eq(schema.apiKeys.userId, user.id)),
|
||||
)
|
||||
.get();
|
||||
|
||||
if (!existing) {
|
||||
|
||||
@@ -301,3 +301,105 @@ describe("tool and pipeline permission enforcement", () => {
|
||||
expect(res.statusCode).toBe(200);
|
||||
});
|
||||
});
|
||||
|
||||
describe("API key ownership scoping", () => {
|
||||
let userAToken: string;
|
||||
let userBToken: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
// Create user A
|
||||
await testApp.app.inject({
|
||||
method: "POST",
|
||||
url: "/api/auth/register",
|
||||
headers: { authorization: `Bearer ${adminToken}` },
|
||||
payload: { username: "keyuserA", password: "TestPass1", role: "user" },
|
||||
});
|
||||
db.update(schema.users)
|
||||
.set({ mustChangePassword: false })
|
||||
.where(eq(schema.users.username, "keyuserA"))
|
||||
.run();
|
||||
const loginA = await testApp.app.inject({
|
||||
method: "POST",
|
||||
url: "/api/auth/login",
|
||||
payload: { username: "keyuserA", password: "TestPass1" },
|
||||
});
|
||||
userAToken = JSON.parse(loginA.body).token;
|
||||
|
||||
// Create user B
|
||||
await testApp.app.inject({
|
||||
method: "POST",
|
||||
url: "/api/auth/register",
|
||||
headers: { authorization: `Bearer ${adminToken}` },
|
||||
payload: { username: "keyuserB", password: "TestPass1", role: "user" },
|
||||
});
|
||||
db.update(schema.users)
|
||||
.set({ mustChangePassword: false })
|
||||
.where(eq(schema.users.username, "keyuserB"))
|
||||
.run();
|
||||
const loginB = await testApp.app.inject({
|
||||
method: "POST",
|
||||
url: "/api/auth/login",
|
||||
payload: { username: "keyuserB", password: "TestPass1" },
|
||||
});
|
||||
userBToken = JSON.parse(loginB.body).token;
|
||||
});
|
||||
|
||||
it("user A cannot see user B's API keys", async () => {
|
||||
// User A creates a key
|
||||
await testApp.app.inject({
|
||||
method: "POST",
|
||||
url: "/api/v1/api-keys",
|
||||
headers: { authorization: `Bearer ${userAToken}` },
|
||||
payload: { name: "A-key" },
|
||||
});
|
||||
|
||||
// User B creates a key
|
||||
await testApp.app.inject({
|
||||
method: "POST",
|
||||
url: "/api/v1/api-keys",
|
||||
headers: { authorization: `Bearer ${userBToken}` },
|
||||
payload: { name: "B-key" },
|
||||
});
|
||||
|
||||
// User A lists keys - should only see their own
|
||||
const res = await testApp.app.inject({
|
||||
method: "GET",
|
||||
url: "/api/v1/api-keys",
|
||||
headers: { authorization: `Bearer ${userAToken}` },
|
||||
});
|
||||
const body = JSON.parse(res.body);
|
||||
expect(body.apiKeys.every((k: any) => k.name === "A-key")).toBe(true);
|
||||
expect(body.apiKeys.some((k: any) => k.name === "B-key")).toBe(false);
|
||||
});
|
||||
|
||||
it("admin can see all API keys", async () => {
|
||||
const res = await testApp.app.inject({
|
||||
method: "GET",
|
||||
url: "/api/v1/api-keys",
|
||||
headers: { authorization: `Bearer ${adminToken}` },
|
||||
});
|
||||
const body = JSON.parse(res.body);
|
||||
// Admin should see keys from both users
|
||||
expect(body.apiKeys.some((k: any) => k.name === "A-key")).toBe(true);
|
||||
expect(body.apiKeys.some((k: any) => k.name === "B-key")).toBe(true);
|
||||
});
|
||||
|
||||
it("user A cannot delete user B's API key", async () => {
|
||||
// Get user B's keys
|
||||
const listRes = await testApp.app.inject({
|
||||
method: "GET",
|
||||
url: "/api/v1/api-keys",
|
||||
headers: { authorization: `Bearer ${userBToken}` },
|
||||
});
|
||||
const bKeys = JSON.parse(listRes.body).apiKeys;
|
||||
if (bKeys.length === 0) throw new Error("Expected user B to have keys");
|
||||
|
||||
// User A tries to delete user B's key
|
||||
const res = await testApp.app.inject({
|
||||
method: "DELETE",
|
||||
url: `/api/v1/api-keys/${bKeys[0].id}`,
|
||||
headers: { authorization: `Bearer ${userAToken}` },
|
||||
});
|
||||
expect(res.statusCode).toBe(404);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user