diff --git a/src/web/api/auth.ts b/src/web/api/auth.ts index 68fc323..16e3d1f 100644 --- a/src/web/api/auth.ts +++ b/src/web/api/auth.ts @@ -3,6 +3,7 @@ import type { MusicProvider } from "../../music/provider.js"; import { YouTubeProvider } from "../../music/youtube.js"; import type { CookieStore } from "../../music/auth.js"; import type { Logger } from "../../logger.js"; +import { requirePermission } from "../middleware/requirePermission.js"; export function createAuthRouter( neteaseProvider: MusicProvider, @@ -35,7 +36,7 @@ export function createAuthRouter( } }); - router.post("/qrcode", async (req, res) => { + router.post("/qrcode", requirePermission("platform.auth"), async (req, res) => { try { const { platform } = req.body; const provider = getProvider(platform); @@ -77,7 +78,7 @@ export function createAuthRouter( } }); - router.post("/sms/send", async (req, res) => { + router.post("/sms/send", requirePermission("platform.auth"), async (req, res) => { try { const { phone } = req.body; if (!phone) { @@ -97,7 +98,7 @@ export function createAuthRouter( } }); - router.post("/sms/verify", async (req, res) => { + router.post("/sms/verify", requirePermission("platform.auth"), async (req, res) => { try { const { phone, code } = req.body; if (!phone || !code) { @@ -118,7 +119,7 @@ export function createAuthRouter( } }); - router.post("/cookie", (req, res) => { + router.post("/cookie", requirePermission("platform.auth"), (req, res) => { const { platform, cookie } = req.body; if (!cookie) { res.status(400).json({ error: "cookie is required" }); diff --git a/src/web/api/bot.ts b/src/web/api/bot.ts index 51a417b..e22b1e3 100755 --- a/src/web/api/bot.ts +++ b/src/web/api/bot.ts @@ -5,6 +5,7 @@ import { saveConfig } from "../../data/config.js"; import type { Logger } from "../../logger.js"; import type { BotDatabase } from "../../data/database.js"; import type { AvatarStore } from "../../data/avatars.js"; +import { requirePermission, requireBotAccess } from "../middleware/requirePermission.js"; export function createBotRouter( botManager: BotManager, @@ -62,7 +63,7 @@ export function createBotRouter( res.send(buf); }); - router.put("/:id/avatar", (req, res) => { + router.put("/:id/avatar", requirePermission("bot.manage"), requireBotAccess("id"), (req, res) => { const exists = botManager.getBot(req.params.id) || botDb.getBotInstances().some((b) => b.id === req.params.id); @@ -96,7 +97,7 @@ export function createBotRouter( res.json({ path: rel }); }); - router.delete("/:id/avatar", (req, res) => { + router.delete("/:id/avatar", requirePermission("bot.manage"), requireBotAccess("id"), (req, res) => { const path = botDb.getCustomAvatarPath(req.params.id); if (path) avatarStore.remove(path); botDb.setCustomAvatarPath(req.params.id, null); @@ -104,7 +105,7 @@ export function createBotRouter( res.status(204).end(); }); - router.post("/", async (req, res) => { + router.post("/", requirePermission("bot.manage"), async (req, res) => { try { const { name, @@ -140,7 +141,7 @@ export function createBotRouter( }); // Update bot config (must be stopped first to apply connection changes) - router.put("/:id", async (req, res) => { + router.put("/:id", requirePermission("bot.manage"), requireBotAccess("id"), async (req, res) => { try { const bot = botManager.getBot(req.params.id); if (!bot) { @@ -159,7 +160,7 @@ export function createBotRouter( } }); - router.delete("/:id", async (req, res) => { + router.delete("/:id", requirePermission("bot.manage"), requireBotAccess("id"), async (req, res) => { try { await botManager.removeBot(req.params.id); res.json({ success: true }); @@ -168,7 +169,7 @@ export function createBotRouter( } }); - router.post("/:id/start", async (req, res) => { + router.post("/:id/start", requirePermission("bot.manage"), requireBotAccess("id"), async (req, res) => { try { await botManager.startBot(req.params.id); res.json({ success: true }); @@ -177,7 +178,7 @@ export function createBotRouter( } }); - router.post("/:id/stop", (req, res) => { + router.post("/:id/stop", requirePermission("bot.manage"), requireBotAccess("id"), (req, res) => { try { botManager.stopBot(req.params.id); res.json({ success: true }); @@ -192,7 +193,7 @@ export function createBotRouter( }); // POST /api/bot/settings — 保存全局 bot 行为设置 - router.post("/settings", (req, res) => { + router.post("/settings", requirePermission("bot.manage"), (req, res) => { const { idleTimeoutMinutes } = req.body; if (typeof idleTimeoutMinutes !== "number" || idleTimeoutMinutes < 0) { res.status(400).json({ error: "idleTimeoutMinutes must be a non-negative number" }); diff --git a/src/web/api/music.ts b/src/web/api/music.ts index edf9c04..1e8a1a5 100644 --- a/src/web/api/music.ts +++ b/src/web/api/music.ts @@ -2,6 +2,7 @@ import { Router } from "express"; import type { MusicProvider } from "../../music/provider.js"; import { YouTubeProvider } from "../../music/youtube.js"; import type { Logger } from "../../logger.js"; +import { requirePermission } from "../middleware/requirePermission.js"; export function createMusicRouter( neteaseProvider: MusicProvider, @@ -217,7 +218,7 @@ export function createMusicRouter( }); // Set quality - router.post("/quality", (req, res) => { + router.post("/quality", requirePermission("quality"), (req, res) => { const { quality, platform } = req.body; if (!quality) { res.status(400).json({ error: "quality is required" }); diff --git a/src/web/api/permissions-enforcement.test.ts b/src/web/api/permissions-enforcement.test.ts new file mode 100644 index 0000000..42c1174 --- /dev/null +++ b/src/web/api/permissions-enforcement.test.ts @@ -0,0 +1,237 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import express from "express"; +import request from "supertest"; +import pino from "pino"; +import { createPlayerRouter } from "./player.js"; +import { createBotRouter } from "./bot.js"; +import { createAuthRouter } from "./auth.js"; +import { createMusicRouter } from "./music.js"; + +const logger = pino({ level: "silent" }); + +// --- minimal stubs -------------------------------------------------------- + +const ALLOWED_BOT = "bot-allowed"; + +// A fake bot whose methods all no-op / return benign values so the real +// handlers run to completion without 500ing. We only assert that the +// permission/bot-access gate let the request THROUGH (status !== 403). +function makeFakeBot(id: string) { + return { + id, + executeCommand: async () => "ok", + getStatus: () => ({ id }), + getQueue: () => [], + getProfileManager: () => ({ getConfig: () => ({}), updateConfig: () => {}, setCustomAvatar: () => {} }), + }; +} + +function makeBotManager() { + const bot = makeFakeBot(ALLOWED_BOT); + return { + getBot: (id: string) => (id === ALLOWED_BOT ? bot : undefined), + getAllBots: () => [bot], + getBotConfig: () => undefined, + createBot: async () => bot, + updateBot: () => {}, + removeBot: async () => {}, + startBot: async () => {}, + stopBot: () => {}, + } as any; +} + +function makeProvider() { + return { + platform: "netease", + getQuality: () => "high", + setQuality: () => {}, + getAuthStatus: async () => ({ loggedIn: false }), + getQrCode: async () => ({ key: "k", url: "u" }), + getCookie: () => "c", + setCookie: () => {}, + search: async () => ({ songs: [], albums: [], playlists: [] }), + } as any; +} + +// Build one app mounting all four real routers, with req.user injected by a +// middleware placed BEFORE the routers (mimicking what requireAuth does). +function makeApp(user: any) { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { (req as any).user = user; next(); }); + + const botManager = makeBotManager(); + const provider = makeProvider(); + + app.use("/api/player", createPlayerRouter(botManager, logger)); + app.use( + "/api/bot", + createBotRouter( + botManager, + { idleTimeoutMinutes: 0 } as any, + "/tmp/config.json", + logger, + { getBotInstances: () => [], getCustomAvatarPath: () => null, setCustomAvatarPath: () => {} } as any, + { read: () => null, write: () => "x", remove: () => {} } as any, + ), + ); + app.use("/api/auth", createAuthRouter(provider, provider, provider, logger)); + app.use("/api/music", createMusicRouter(provider, provider, provider, logger)); + + return app; +} + +const member = (caps: string[], bots: "all" | string[]) => ({ + id: "u1", + username: "alice", + role: "member" as const, + capabilities: new Set(caps), + bots: bots === "all" ? ("all" as const) : new Set(bots), +}); + +const admin = { + id: "a", + username: "admin", + role: "admin" as const, + capabilities: new Set(), + bots: "all" as const, +}; + +describe("permission enforcement on action routes", () => { + describe("player.control", () => { + it("403 for member WITHOUT player.control", async () => { + const app = makeApp(member([], [ALLOWED_BOT])); + const res = await request(app).post(`/api/player/${ALLOWED_BOT}/pause`); + expect(res.status).toBe(403); + }); + + it("NOT 403 for member WITH player.control + bot in allow-list", async () => { + const app = makeApp(member(["player.control"], [ALLOWED_BOT])); + const res = await request(app).post(`/api/player/${ALLOWED_BOT}/pause`); + expect(res.status).not.toBe(403); + }); + + it("403 for member WITH player.control but bot NOT in allow-list", async () => { + const app = makeApp(member(["player.control"], ["other-bot"])); + const res = await request(app).post(`/api/player/${ALLOWED_BOT}/pause`); + expect(res.status).toBe(403); + }); + }); + + describe("player.queue", () => { + it("403 for member WITHOUT player.queue", async () => { + const app = makeApp(member(["player.control"], [ALLOWED_BOT])); + const res = await request(app).post(`/api/player/${ALLOWED_BOT}/clear`); + expect(res.status).toBe(403); + }); + + it("NOT 403 for member WITH player.queue", async () => { + const app = makeApp(member(["player.queue"], [ALLOWED_BOT])); + const res = await request(app).post(`/api/player/${ALLOWED_BOT}/clear`); + expect(res.status).not.toBe(403); + }); + }); + + describe("bot.manage", () => { + it("403 for member WITHOUT bot.manage on POST /api/bot", async () => { + const app = makeApp(member([], "all")); + const res = await request(app) + .post("/api/bot") + .send({ name: "n", serverAddress: "s", nickname: "nick" }); + expect(res.status).toBe(403); + }); + + it("NOT 403 for member WITH bot.manage on POST /api/bot", async () => { + const app = makeApp(member(["bot.manage"], "all")); + const res = await request(app) + .post("/api/bot") + .send({ name: "n", serverAddress: "s", nickname: "nick" }); + expect(res.status).not.toBe(403); + }); + + it("403 for member WITH bot.manage but bot NOT in allow-list on POST /api/bot/:id/start", async () => { + const app = makeApp(member(["bot.manage"], ["other-bot"])); + const res = await request(app).post(`/api/bot/${ALLOWED_BOT}/start`); + expect(res.status).toBe(403); + }); + + it("NOT 403 for member WITH bot.manage + bot in allow-list on POST /api/bot/:id/start", async () => { + const app = makeApp(member(["bot.manage"], [ALLOWED_BOT])); + const res = await request(app).post(`/api/bot/${ALLOWED_BOT}/start`); + expect(res.status).not.toBe(403); + }); + }); + + describe("platform.auth", () => { + it("403 for member WITHOUT platform.auth on POST /api/auth/cookie", async () => { + const app = makeApp(member([], "all")); + const res = await request(app).post("/api/auth/cookie").send({ cookie: "c" }); + expect(res.status).toBe(403); + }); + + it("NOT 403 for member WITH platform.auth on POST /api/auth/cookie", async () => { + const app = makeApp(member(["platform.auth"], "all")); + const res = await request(app).post("/api/auth/cookie").send({ cookie: "c" }); + expect(res.status).not.toBe(403); + }); + }); + + describe("quality", () => { + it("403 for member WITHOUT quality on POST /api/music/quality", async () => { + const app = makeApp(member([], "all")); + const res = await request(app).post("/api/music/quality").send({ quality: "high" }); + expect(res.status).toBe(403); + }); + + it("NOT 403 for member WITH quality on POST /api/music/quality", async () => { + const app = makeApp(member(["quality"], "all")); + const res = await request(app).post("/api/music/quality").send({ quality: "high" }); + expect(res.status).not.toBe(403); + }); + }); + + describe("read-only routes stay open", () => { + it("GET /api/auth/status not gated", async () => { + const app = makeApp(member([], "all")); + const res = await request(app).get("/api/auth/status"); + expect(res.status).not.toBe(403); + }); + + it("GET /api/music/quality not gated", async () => { + const app = makeApp(member([], "all")); + const res = await request(app).get("/api/music/quality"); + expect(res.status).not.toBe(403); + }); + + it("GET /api/bot not gated", async () => { + const app = makeApp(member([], "all")); + const res = await request(app).get("/api/bot"); + expect(res.status).not.toBe(403); + }); + }); + + describe("admin bypasses every gate", () => { + let app: express.Express; + beforeEach(() => { app = makeApp(admin); }); + + it("player.control", async () => { + expect((await request(app).post(`/api/player/${ALLOWED_BOT}/pause`)).status).not.toBe(403); + }); + it("player.queue", async () => { + expect((await request(app).post(`/api/player/${ALLOWED_BOT}/clear`)).status).not.toBe(403); + }); + it("bot.manage POST /api/bot", async () => { + const res = await request(app).post("/api/bot").send({ name: "n", serverAddress: "s", nickname: "nick" }); + expect(res.status).not.toBe(403); + }); + it("bot.manage POST /api/bot/:id/start", async () => { + expect((await request(app).post(`/api/bot/${ALLOWED_BOT}/start`)).status).not.toBe(403); + }); + it("platform.auth POST /api/auth/cookie", async () => { + expect((await request(app).post("/api/auth/cookie").send({ cookie: "c" })).status).not.toBe(403); + }); + it("quality POST /api/music/quality", async () => { + expect((await request(app).post("/api/music/quality").send({ quality: "high" })).status).not.toBe(403); + }); + }); +}); diff --git a/src/web/api/player.ts b/src/web/api/player.ts index 4f0930b..373a32a 100644 --- a/src/web/api/player.ts +++ b/src/web/api/player.ts @@ -4,6 +4,7 @@ import type { BotDatabase } from "../../data/database.js"; import type { MusicProvider } from "../../music/provider.js"; import type { Logger } from "../../logger.js"; import { parseCommand } from "../../bot/commands.js"; +import { requirePermission, requireBotAccess } from "../middleware/requirePermission.js"; export function createPlayerRouter( botManager: BotManager, @@ -25,6 +26,8 @@ export function createPlayerRouter( next(); }); + router.use("/:botId", requireBotAccess("botId")); + /** Map API platform string to the corresponding command flag. */ const platformFlag = (platform: unknown): string => { if (platform === "bilibili") return "-b"; @@ -33,7 +36,7 @@ export function createPlayerRouter( return ""; }; - router.post("/:botId/play", async (req, res) => { + router.post("/:botId/play", requirePermission("player.control"), async (req, res) => { try { const bot = (req as any).bot; const { query, platform } = req.body; @@ -53,7 +56,7 @@ export function createPlayerRouter( } }); - router.post("/:botId/add", async (req, res) => { + router.post("/:botId/add", requirePermission("player.queue"), async (req, res) => { try { const bot = (req as any).bot; const { query, platform } = req.body; @@ -80,14 +83,14 @@ export function createPlayerRouter( } }; - router.post("/:botId/pause", simpleCommand("!pause")); - router.post("/:botId/resume", simpleCommand("!resume")); - router.post("/:botId/next", simpleCommand("!next")); - router.post("/:botId/prev", simpleCommand("!prev")); - router.post("/:botId/stop", simpleCommand("!stop")); - router.post("/:botId/clear", simpleCommand("!clear")); + router.post("/:botId/pause", requirePermission("player.control"), simpleCommand("!pause")); + router.post("/:botId/resume", requirePermission("player.control"), simpleCommand("!resume")); + router.post("/:botId/next", requirePermission("player.control"), simpleCommand("!next")); + router.post("/:botId/prev", requirePermission("player.control"), simpleCommand("!prev")); + router.post("/:botId/stop", requirePermission("player.control"), simpleCommand("!stop")); + router.post("/:botId/clear", requirePermission("player.queue"), simpleCommand("!clear")); - router.post("/:botId/volume", async (req, res) => { + router.post("/:botId/volume", requirePermission("player.control"), async (req, res) => { try { const bot = (req as any).bot; const { volume } = req.body; @@ -115,7 +118,7 @@ export function createPlayerRouter( const VALID_MODES = new Set(["seq", "loop", "random", "rloop"]); - router.post("/:botId/mode", async (req, res) => { + router.post("/:botId/mode", requirePermission("player.control"), async (req, res) => { try { const bot = (req as any).bot; const { mode } = req.body; @@ -140,7 +143,7 @@ export function createPlayerRouter( }); // Seek to position - router.post("/:botId/seek", async (req, res) => { + router.post("/:botId/seek", requirePermission("player.control"), async (req, res) => { try { const bot = (req as any).bot; const { position } = req.body; // seconds @@ -164,7 +167,7 @@ export function createPlayerRouter( res.json({ queue: bot.getQueue(), status: bot.getStatus() }); }); - router.delete("/:botId/queue/:index", async (req, res) => { + router.delete("/:botId/queue/:index", requirePermission("player.queue"), async (req, res) => { try { const bot = (req as any).bot; const cmd = parseCommand(`!remove ${req.params.index}`, "!")!; @@ -176,7 +179,7 @@ export function createPlayerRouter( }); // Jump to a specific index in the queue (without clearing it) - router.post("/:botId/play-at", async (req, res) => { + router.post("/:botId/play-at", requirePermission("player.control"), async (req, res) => { try { const bot = (req as any).bot; const { index } = req.body; @@ -210,7 +213,7 @@ export function createPlayerRouter( } }); - router.post("/:botId/playlist", async (req, res) => { + router.post("/:botId/playlist", requirePermission("player.queue"), async (req, res) => { try { const bot = (req as any).bot; const { playlistId, platform } = req.body; @@ -227,7 +230,7 @@ export function createPlayerRouter( // Play a playlist by ID — stores metadata only, resolves URL for first song // Respects current play mode (random = pick random first song) - router.post("/:botId/play-playlist", async (req, res) => { + router.post("/:botId/play-playlist", requirePermission("player.control"), async (req, res) => { try { const bot = (req as any).bot; const { playlistId, platform } = req.body; @@ -314,7 +317,7 @@ export function createPlayerRouter( }); // Play an album by ID — mirrors play-playlist but calls getAlbumSongs - router.post("/:botId/play-album", async (req, res) => { + router.post("/:botId/play-album", requirePermission("player.control"), async (req, res) => { try { const bot = (req as any).bot; const { albumId, platform } = req.body; @@ -386,7 +389,7 @@ export function createPlayerRouter( }); // Play a single song by ID — resolves URL on demand - router.post("/:botId/play-song", async (req, res) => { + router.post("/:botId/play-song", requirePermission("player.control"), async (req, res) => { try { const bot = (req as any).bot; const { song } = req.body; @@ -414,7 +417,7 @@ export function createPlayerRouter( // Insert a single song to play right after the current one. // If nothing is playing, behaves like /play-song (start immediately). - router.post("/:botId/play-next-song", async (req, res) => { + router.post("/:botId/play-next-song", requirePermission("player.control"), async (req, res) => { try { const bot = (req as any).bot; const { song } = req.body; @@ -452,7 +455,7 @@ export function createPlayerRouter( } }); - router.post("/:botId/add-song", async (req, res) => { + router.post("/:botId/add-song", requirePermission("player.queue"), async (req, res) => { try { const bot = (req as any).bot; const { song } = req.body; @@ -480,7 +483,7 @@ export function createPlayerRouter( }); // Add a song to queue by ID — metadata only - router.post("/:botId/add-by-id", async (req, res) => { + router.post("/:botId/add-by-id", requirePermission("player.queue"), async (req, res) => { try { const bot = (req as any).bot; const { songId, platform } = req.body; @@ -518,7 +521,7 @@ export function createPlayerRouter( res.json(bot.getProfileManager().getConfig()); }); - router.put("/:botId/profile", (req, res) => { + router.put("/:botId/profile", requirePermission("bot.manage"), (req, res) => { try { const bot = (req as any).bot; const pm = bot.getProfileManager(); diff --git a/src/web/middleware/requirePermission.ts b/src/web/middleware/requirePermission.ts index 5adfeb5..831c734 100644 --- a/src/web/middleware/requirePermission.ts +++ b/src/web/middleware/requirePermission.ts @@ -1,18 +1,23 @@ import type { Request, Response, NextFunction, RequestHandler } from "express"; -export function requirePermission(capability: string): RequestHandler { - return (req: Request, res: Response, next: NextFunction) => { +// Generic over the route-param shape (`P`) so Express can keep inferring +// `req.params` from the route string (e.g. `/:id` → `{ id: string }`) when +// these are passed as a per-route middleware argument. Pinning the default +// `ParamsDictionary` here would otherwise force the broad +// `string | string[]` param overload on every route they guard. +export function requirePermission

>(capability: string): RequestHandler

{ + return (req: Request

, res: Response, next: NextFunction) => { if (!req.user) { res.status(401).json({ error: "unauthenticated" }); return; } if (req.user.role === "admin" || req.user.capabilities?.has(capability)) { next(); return; } res.status(403).json({ error: "forbidden" }); }; } -export function requireBotAccess(paramName = "botId"): RequestHandler { - return (req: Request, res: Response, next: NextFunction) => { +export function requireBotAccess

>(paramName = "botId"): RequestHandler

{ + return (req: Request

, res: Response, next: NextFunction) => { if (!req.user) { res.status(401).json({ error: "unauthenticated" }); return; } if (req.user.role === "admin" || req.user.bots === "all") { next(); return; } - const botId = req.params[paramName]; + const botId = (req.params as Record)[paramName]; if (typeof botId === "string" && req.user.bots instanceof Set && req.user.bots.has(botId)) { next(); return; } res.status(403).json({ error: "forbidden" }); };