fix(guest): deny favorites + auth-status reads to guests; UI polish

Consolidated fix wave from the final whole-branch review of guest mode.

- FIX 1 (critical): gate /api/favorites mount with requireNotGuest — the
  router keys off req.user.id (shared __guest__ principal), so guests could
  read/write a shared favorites bucket. Added focused guest-deny tests.
- FIX 2: gate GET /api/auth/status and /api/auth/qrcode/status with
  requireNotGuest so config reads no longer leak to guests.
- FIX 3: requireAuthInline in createSessionRouter now rejects guest sessions
  with 401 once guest mode is disabled (mirrors createRequireAuth), so /me
  stops returning guest data after an admin disables the feature.
- FIX 4: Login guest button now sits BELOW the card (auth-page flex-direction
  column + guest-btn width 360px) instead of beside it.
- FIX 5: mobile mini-player transport buttons in App.vue are now per-button
  gated for guests (prev/play/next/mode/volume), mirroring Player.vue.
- FIX 6: refreshed stale "gated on player.control" seek comments in Player.vue
  and relabeled the now-stale quality-GET test.

npm test: 354/354 pass. npm run build: tsc + vue-tsc + vite all green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
saopig1andClaude Opus 4.8 committed 2026-06-25 12:44:07 +08:00
1 parent e47fc76529
commit f142c514cd
8 files changed
+113 -12

No files matched your search

+3 -2
View File
@@ -4,6 +4,7 @@ import { YouTubeProvider } from "../../music/youtube.js";
import type { CookieStore } from "../../music/auth.js"; import type { CookieStore } from "../../music/auth.js";
import type { Logger } from "../../logger.js"; import type { Logger } from "../../logger.js";
import { requirePermission } from "../middleware/requirePermission.js"; import { requirePermission } from "../middleware/requirePermission.js";
import { requireNotGuest } from "../middleware/requireNotGuest.js";
export function createAuthRouter( export function createAuthRouter(
neteaseProvider: MusicProvider, neteaseProvider: MusicProvider,
@@ -23,7 +24,7 @@ export function createAuthRouter(
return platform === "qq" ? qqProvider : neteaseProvider; return platform === "qq" ? qqProvider : neteaseProvider;
} }
router.get("/status", async (req, res) => { router.get("/status", requireNotGuest, async (req, res) => {
try { try {
const platform = req.query.platform as string; const platform = req.query.platform as string;
const provider = getProvider(platform); const provider = getProvider(platform);
@@ -49,7 +50,7 @@ export function createAuthRouter(
} }
}); });
router.get("/qrcode/status", async (req, res) => { router.get("/qrcode/status", requireNotGuest, async (req, res) => {
try { try {
const { key, platform } = req.query; const { key, platform } = req.query;
if (!key) { if (!key) {
+54 -1
View File
@@ -6,6 +6,8 @@ import { createPlayerRouter } from "./player.js";
import { createBotRouter } from "./bot.js"; import { createBotRouter } from "./bot.js";
import { createAuthRouter } from "./auth.js"; import { createAuthRouter } from "./auth.js";
import { createMusicRouter } from "./music.js"; import { createMusicRouter } from "./music.js";
import { createFavoritesRouter } from "./favorites.js";
import { requireNotGuest } from "../middleware/requireNotGuest.js";
const logger = pino({ level: "silent" }); const logger = pino({ level: "silent" });
@@ -204,7 +206,7 @@ describe("permission enforcement on action routes", () => {
expect(res.status).not.toBe(403); expect(res.status).not.toBe(403);
}); });
it("GET /api/music/quality not gated", async () => { it("GET /api/music/quality readable by members, denied to guests", async () => {
const app = makeApp(member([], "all")); const app = makeApp(member([], "all"));
const res = await request(app).get("/api/music/quality"); const res = await request(app).get("/api/music/quality");
expect(res.status).not.toBe(403); expect(res.status).not.toBe(403);
@@ -215,6 +217,12 @@ describe("permission enforcement on action routes", () => {
const res = await request(app).get("/api/bot"); const res = await request(app).get("/api/bot");
expect(res.status).not.toBe(403); expect(res.status).not.toBe(403);
}); });
it("GET /api/auth/status and /api/auth/qrcode/status are 403 for guests", async () => {
const app = makeApp(guest());
expect((await request(app).get("/api/auth/status")).status).toBe(403);
expect((await request(app).get("/api/auth/qrcode/status?key=k")).status).toBe(403);
});
}); });
describe("admin bypasses every gate", () => { describe("admin bypasses every gate", () => {
@@ -368,3 +376,48 @@ describe("guest enforcement on player routes", () => {
expect((await request(m).post(`/api/player/${ALLOWED_BOT}/add-song`).send({ song: SONG })).status).not.toBe(403); expect((await request(m).post(`/api/player/${ALLOWED_BOT}/add-song`).send({ song: SONG })).status).not.toBe(403);
}); });
}); });
// --------------------------------------------------------------------------
// Favorites are member-only: the router keys everything off req.user.id and
// all guests share the __guest__ principal, so a guest must never reach it.
// server.ts gates the mount with requireNotGuest; we mirror that mount here
// and assert a guest gets 403 (the requireNotGuest guard runs before any
// handler, so the fake database is never touched).
// --------------------------------------------------------------------------
function makeFavoritesApp(user: any) {
const app = express();
app.use(express.json());
app.use((req, _res, next) => { (req as any).user = user; next(); });
const fakeDb = {
getFavorites: () => [],
addFavorite: () => {},
removeFavorite: () => {},
isFavorited: () => false,
} as any;
app.use("/api/favorites", requireNotGuest, createFavoritesRouter(fakeDb, logger));
return app;
}
describe("favorites are denied to guests", () => {
it("403 for a guest on GET /api/favorites", async () => {
const app = makeFavoritesApp(guest());
expect((await request(app).get("/api/favorites")).status).toBe(403);
});
it("403 for a guest on GET /api/favorites/check", async () => {
const app = makeFavoritesApp(guest());
expect((await request(app).get("/api/favorites/check?platform=netease&playlistId=x")).status).toBe(403);
});
it("403 for a guest on POST /api/favorites", async () => {
const app = makeFavoritesApp(guest());
const res = await request(app).post("/api/favorites").send({ platform: "netease", playlistId: "x", name: "n" });
expect(res.status).toBe(403);
});
it("NOT 403 for a member on GET /api/favorites", async () => {
const app = makeFavoritesApp(member([], "all"));
expect((await request(app).get("/api/favorites")).status).not.toBe(403);
});
});
+33
View File
@@ -236,4 +236,37 @@ describe("session router — guest mode", () => {
const res = await request(app).get("/api/session/needs-setup"); const res = await request(app).get("/api/session/needs-setup");
expect(res.body.guestAllowed).toBe(true); expect(res.body.guestAllowed).toBe(true);
}); });
it("GET /me returns 401 for a guest session once guest mode is disabled", async () => {
// Build an app whose guest config can be toggled at runtime, mirroring an
// admin flipping the setting mid-session (requireAuthInline must reject).
botDb = createDatabase(":memory:");
const users = createUserStore(botDb.db);
const sessions = createSessionStore(botDb.db);
const audit = createAuditStore(botDb.db);
const permissions = createPermissionStore(botDb.db);
const guestCfg: GuestModeConfig = {
enabled: true,
bots: getDefaultConfig().guestMode.bots,
permissions: getDefaultConfig().guestMode.permissions,
};
const app = express();
app.use(express.json());
app.use(cookieParser());
app.use(
"/api/session",
createSessionRouter(users, sessions, audit, pino({ level: "silent" }), permissions, () => guestCfg)
);
const login = await request(app).post("/api/session/guest");
expect(login.status).toBe(200);
const cookie = login.headers["set-cookie"];
// While enabled, /me works for the guest.
expect((await request(app).get("/api/session/me").set("Cookie", cookie)).status).toBe(200);
// Admin disables guest mode → the in-flight guest session is now invalid.
guestCfg.enabled = false;
expect((await request(app).get("/api/session/me").set("Cookie", cookie)).status).toBe(401);
});
}); });
+7
View File
@@ -65,6 +65,13 @@ export function createSessionRouter(
res.status(401).json({ error: "unauthenticated" }); res.status(401).json({ error: "unauthenticated" });
return; return;
} }
// A guest session is only valid while guest mode is enabled. Disabling it
// immediately invalidates any in-flight guest sessions (mirrors createRequireAuth).
if (result.role === "guest" && !getGuestConfig().enabled) {
clearSessionCookie(res);
res.status(401).json({ error: "unauthenticated" });
return;
}
req.user = { id: result.userId, username: result.username, role: result.role }; req.user = { id: result.userId, username: result.username, role: result.role };
const token = extractSessionToken(req.headers.cookie); const token = extractSessionToken(req.headers.cookie);
if (token) setSessionCookie(res, token); if (token) setSessionCookie(res, token);
+2 -1
View File
@@ -25,6 +25,7 @@ import { createSessionStore } from "../data/sessions.js";
import { createPermissionStore } from "../data/permissions.js"; import { createPermissionStore } from "../data/permissions.js";
import { createRequireAuth } from "./middleware/requireAuth.js"; import { createRequireAuth } from "./middleware/requireAuth.js";
import { requireAdmin } from "./middleware/requireAdmin.js"; import { requireAdmin } from "./middleware/requireAdmin.js";
import { requireNotGuest } from "./middleware/requireNotGuest.js";
import { csrfOriginCheck } from "./middleware/csrf.js"; import { csrfOriginCheck } from "./middleware/csrf.js";
import { createRateLimit } from "./middleware/rateLimit.js"; import { createRateLimit } from "./middleware/rateLimit.js";
import { validateSessionFromHeaders } from "./auth/validateSession.js"; import { validateSessionFromHeaders } from "./auth/validateSession.js";
@@ -126,7 +127,7 @@ export function createWebServer(options: WebServerOptions): WebServer {
"/api/auth", "/api/auth",
createAuthRouter(options.neteaseProvider, options.qqProvider, options.bilibiliProvider, logger, options.cookieStore) createAuthRouter(options.neteaseProvider, options.qqProvider, options.bilibiliProvider, logger, options.cookieStore)
); );
app.use("/api/favorites", createFavoritesRouter(options.database, logger)); app.use("/api/favorites", requireNotGuest, createFavoritesRouter(options.database, logger));
// admin-only routes // admin-only routes
app.use("/api/users", requireAdmin, createUsersRouter(users, sessions, audit, logger, permissions)); app.use("/api/users", requireAdmin, createUsersRouter(users, sessions, audit, logger, permissions));
+10 -5
View File
@@ -19,22 +19,22 @@
<div class="m-player-artist">{{ currentSong.artist }}</div> <div class="m-player-artist">{{ currentSong.artist }}</div>
</div> </div>
<div class="m-player-controls" @click.stop> <div class="m-player-controls" @click.stop>
<button class="m-player-btn" @click="playerStore.prev()"> <button v-if="can('player.control')" class="m-player-btn" @click="playerStore.prev()">
<Icon icon="mdi:skip-previous" /> <Icon icon="mdi:skip-previous" />
</button> </button>
<button class="m-player-btn" @click="playerStore.isPlaying ? playerStore.pause() : playerStore.resume()"> <button v-if="canTransport" class="m-player-btn" @click="playerStore.isPlaying ? playerStore.pause() : playerStore.resume()">
<Icon :icon="playerStore.isPlaying ? 'mdi:pause' : 'mdi:play'" /> <Icon :icon="playerStore.isPlaying ? 'mdi:pause' : 'mdi:play'" />
</button> </button>
<button class="m-player-btn" @click="playerStore.next()"> <button v-if="canSkip" class="m-player-btn" @click="playerStore.next()">
<Icon icon="mdi:skip-next" /> <Icon icon="mdi:skip-next" />
</button> </button>
<button class="m-player-btn" @click="cycleMobileMode"> <button v-if="canModeCtl" class="m-player-btn" @click="cycleMobileMode">
<Icon :icon="mobileModeIcon" /> <Icon :icon="mobileModeIcon" />
</button> </button>
<button class="m-player-btn" @click="toggleMobileQueue"> <button class="m-player-btn" @click="toggleMobileQueue">
<Icon icon="mdi:playlist-music" /> <Icon icon="mdi:playlist-music" />
</button> </button>
<button class="m-player-btn" @click="toggleMobileVolume"> <button v-if="canTransport" class="m-player-btn" @click="toggleMobileVolume">
<Icon icon="mdi:volume-high" /> <Icon icon="mdi:volume-high" />
</button> </button>
</div> </div>
@@ -89,6 +89,11 @@ import Queue from './components/Queue.vue';
const playerStore = usePlayerStore(); const playerStore = usePlayerStore();
const session = useSession(); const session = useSession();
const { can, guestCan } = session;
// Mobile mini-player transport gating — mirrors components/Player.vue.
const canTransport = computed(() => can('player.control') || guestCan('transport'));
const canSkip = computed(() => can('player.control') || guestCan('skip'));
const canModeCtl = computed(() => can('player.control') || guestCan('playMode'));
const theme = computed(() => playerStore.theme); const theme = computed(() => playerStore.theme);
const route = useRoute(); const route = useRoute();
const router = useRouter(); const router = useRouter();
+2 -2
View File
@@ -3,7 +3,7 @@
<Queue :open="showQueue" @close="showQueue = false" /> <Queue :open="showQueue" @close="showQueue = false" />
<div class="player-bar frosted-glass"> <div class="player-bar frosted-glass">
<!-- Progress bar (read-only display; seek interaction gated on player.control) --> <!-- Progress bar (read-only display; seek interaction gated on transport / canTransport) -->
<div <div
class="progress-bar-container" class="progress-bar-container"
:class="{ 'no-seek': !canTransport }" :class="{ 'no-seek': !canTransport }"
@@ -147,7 +147,7 @@ function updateProgress() {
} }
async function onProgressClick(e: MouseEvent) { async function onProgressClick(e: MouseEvent) {
if (!canTransport.value) return; // seek requires player.control if (!canTransport.value) return; // seek gated on transport (canTransport)
const bar = progressBarRef.value; const bar = progressBarRef.value;
if (!bar) return; if (!bar) return;
const rect = bar.getBoundingClientRect(); const rect = bar.getBoundingClientRect();
+2 -1
View File
@@ -73,6 +73,7 @@ async function enterAsGuest() {
.auth-page { .auth-page {
min-height: 100vh; min-height: 100vh;
display: flex; display: flex;
flex-direction: column;
align-items: center; align-items: center;
justify-content: center; justify-content: center;
background: var(--bg-primary); background: var(--bg-primary);
@@ -100,7 +101,7 @@ async function enterAsGuest() {
.auth-card button:disabled { opacity: 0.6; cursor: progress; } .auth-card button:disabled { opacity: 0.6; cursor: progress; }
.auth-error { color: #e26a6a; font-size: 13px; margin: 0; } .auth-error { color: #e26a6a; font-size: 13px; margin: 0; }
.guest-btn { .guest-btn {
height: 38px; margin-top: 4px; border-radius: var(--radius-sm); width: 360px; height: 38px; margin-top: 4px; border-radius: var(--radius-sm);
background: transparent; color: var(--text-secondary); background: transparent; color: var(--text-secondary);
border: 1px solid var(--border-color); cursor: pointer; border: 1px solid var(--border-color); cursor: pointer;
} }