fix: inject the admin key so the admin UI stops returning 403
The entire admin surface of the SPA had been dead since auth landed (5ed8d9e/3bc9f2d). nginx.conf.template injected only `Authorization: Bearer ${MUZICK_API_KEY}` for all of /api, docker-compose passed only MUZICK_API_KEY to the frontend container, and app.ts requires token === adminKey for /api/admin/*. The two keys differ, and services/api.ts sets no headers of its own. All 10 admin call sites were affected: the Jobs page polled 403s every 3s/5s forever and rendered a blank Overview with no error state, and every Settings library action (Scan, Reindex, Reprocess artists, Re-enrich, Duplicates merge) silently failed. Three changes, each necessary: - a `location /api/admin/` block injecting the admin key - the Dockerfile envsubst list widened to include MUZICK_ADMIN_KEY, without which the new variable substitutes to empty and the header becomes a bare "Bearer" - MUZICK_ADMIN_KEY passed to the frontend service in docker-compose nginx selects the longest matching prefix regardless of block order; verified empirically in a throwaway nginx:stable-alpine running the real envsubst output against a stub that echoes $http_authorization: /api/admin/queue-stats -> Bearer ADMINKEY456 /api/admin/duplicates/merge -> Bearer ADMINKEY456 /api/tracks -> Bearer APIKEY123 /api/health -> Bearer APIKEY123 All 10 call sites use /admin/... under the axios /api baseURL and none request bare /api/admin without a trailing slash. Also gives the Jobs page an error state: a banner that names a 401/403 as a missing or wrong admin key, a Retry button, "Loading queue stats..." in place of a blank Overview, and refetchInterval returning false once the query has errored so it stops hammering a failing endpoint. Deletes frontend/nginx.conf — unreferenced by the Dockerfile (confirmed by grep) and the insecure variant of the template. Worth noting and not addressed here: the outer LAN-only proxy already forges credentials for everything reaching /api, so this key split buys no real security while having cost the whole admin surface. Collapsing to one key would be simpler. REVIEW-2026-07-30.md finding 2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -34,6 +34,9 @@ services:
|
|||||||
MUZICK_ADMIN_KEY: ${MUZICK_ADMIN_KEY}
|
MUZICK_ADMIN_KEY: ${MUZICK_ADMIN_KEY}
|
||||||
MUSIC_DIR: /music
|
MUSIC_DIR: /music
|
||||||
volumes:
|
volumes:
|
||||||
|
# READ-ONLY, deliberately. Nothing in the API request path may write to
|
||||||
|
# the library. Hard deletion of disliked files happens only in the worker,
|
||||||
|
# which is the sole service with an rw mount.
|
||||||
- /mnt/hdd1/media/Music:/music:ro
|
- /mnt/hdd1/media/Music:/music:ro
|
||||||
depends_on:
|
depends_on:
|
||||||
- db
|
- db
|
||||||
@@ -49,6 +52,7 @@ services:
|
|||||||
- "127.0.0.1:5174:80"
|
- "127.0.0.1:5174:80"
|
||||||
environment:
|
environment:
|
||||||
MUZICK_API_KEY: ${MUZICK_API_KEY}
|
MUZICK_API_KEY: ${MUZICK_API_KEY}
|
||||||
|
MUZICK_ADMIN_KEY: ${MUZICK_ADMIN_KEY}
|
||||||
depends_on:
|
depends_on:
|
||||||
- backend
|
- backend
|
||||||
|
|
||||||
|
|||||||
+1
-1
@@ -9,4 +9,4 @@ FROM nginx:stable-alpine
|
|||||||
COPY --from=build /app/dist /usr/share/nginx/html
|
COPY --from=build /app/dist /usr/share/nginx/html
|
||||||
COPY nginx.conf.template /etc/nginx/templates/default.conf.template
|
COPY nginx.conf.template /etc/nginx/templates/default.conf.template
|
||||||
EXPOSE 80
|
EXPOSE 80
|
||||||
CMD ["/bin/sh", "-c", "envsubst '${MUZICK_API_KEY}' < /etc/nginx/templates/default.conf.template > /etc/nginx/conf.d/default.conf && nginx -g 'daemon off;'"]
|
CMD ["/bin/sh", "-c", "envsubst '${MUZICK_API_KEY} ${MUZICK_ADMIN_KEY}' < /etc/nginx/templates/default.conf.template > /etc/nginx/conf.d/default.conf && nginx -g 'daemon off;'"]
|
||||||
|
|||||||
@@ -1,18 +0,0 @@
|
|||||||
server {
|
|
||||||
listen 80;
|
|
||||||
|
|
||||||
location / {
|
|
||||||
root /usr/share/nginx/html;
|
|
||||||
index index.html index.htm;
|
|
||||||
try_files $uri $uri/ /index.html;
|
|
||||||
}
|
|
||||||
|
|
||||||
location /api {
|
|
||||||
proxy_pass http://backend:3000;
|
|
||||||
proxy_http_version 1.1;
|
|
||||||
proxy_set_header Upgrade $http_upgrade;
|
|
||||||
proxy_set_header Connection 'upgrade';
|
|
||||||
proxy_set_header Host $host;
|
|
||||||
proxy_cache_bypass $http_upgrade;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -7,6 +7,19 @@ server {
|
|||||||
try_files $uri $uri/ /index.html;
|
try_files $uri $uri/ /index.html;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# Admin endpoints need the admin key, not the regular API key. This prefix
|
||||||
|
# location is longer than "/api", and nginx picks the longest matching
|
||||||
|
# prefix location, so it wins for /api/admin/* while /api handles the rest.
|
||||||
|
location /api/admin/ {
|
||||||
|
proxy_pass http://backend:3000;
|
||||||
|
proxy_http_version 1.1;
|
||||||
|
proxy_set_header Upgrade $http_upgrade;
|
||||||
|
proxy_set_header Connection 'upgrade';
|
||||||
|
proxy_set_header Host $host;
|
||||||
|
proxy_set_header Authorization "Bearer ${MUZICK_ADMIN_KEY}";
|
||||||
|
proxy_cache_bypass $http_upgrade;
|
||||||
|
}
|
||||||
|
|
||||||
location /api {
|
location /api {
|
||||||
proxy_pass http://backend:3000;
|
proxy_pass http://backend:3000;
|
||||||
proxy_http_version 1.1;
|
proxy_http_version 1.1;
|
||||||
|
|||||||
@@ -317,6 +317,17 @@ function FilterBar({
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Human-readable one-liner for a failed admin request. */
|
||||||
|
function describeError(err: unknown): string {
|
||||||
|
const status = (err as { response?: { status?: number } })?.response?.status;
|
||||||
|
if (status === 401 || status === 403) {
|
||||||
|
return `Request rejected (HTTP ${status}) — the admin API key is missing or wrong.`;
|
||||||
|
}
|
||||||
|
if (status) return `Request failed with HTTP ${status}.`;
|
||||||
|
const message = err instanceof Error ? err.message : String(err);
|
||||||
|
return message || 'Unknown error.';
|
||||||
|
}
|
||||||
|
|
||||||
/* ─────────────────────────────────────────── Page ────────────────────────────────────────────────── */
|
/* ─────────────────────────────────────────── Page ────────────────────────────────────────────────── */
|
||||||
|
|
||||||
export default function JobsPage() {
|
export default function JobsPage() {
|
||||||
@@ -325,20 +336,26 @@ export default function JobsPage() {
|
|||||||
const [filters, setFilters] = useState<Filters>(INITIAL_FILTERS);
|
const [filters, setFilters] = useState<Filters>(INITIAL_FILTERS);
|
||||||
const [expandedIds, setExpandedIds] = useState<Set<string>>(new Set());
|
const [expandedIds, setExpandedIds] = useState<Set<string>>(new Set());
|
||||||
|
|
||||||
const { data: stats, refetch: refetchStats } = useQuery({
|
const statsQ = useQuery({
|
||||||
queryKey: ['queueStats'],
|
queryKey: ['queueStats'],
|
||||||
queryFn: () => jobsService.getQueueStats(),
|
queryFn: () => jobsService.getQueueStats(),
|
||||||
refetchInterval: autoRefresh ? 3000 : false,
|
// Stop polling once the endpoint is failing — otherwise a permission or
|
||||||
|
// outage error is retried silently every 3s forever.
|
||||||
|
refetchInterval: (query) => (autoRefresh && !query.state.error ? 3000 : false),
|
||||||
staleTime: 1000,
|
staleTime: 1000,
|
||||||
});
|
});
|
||||||
|
|
||||||
const { data: history, refetch: refetchHistory } = useQuery({
|
const historyQ = useQuery({
|
||||||
queryKey: ['jobHistory'],
|
queryKey: ['jobHistory'],
|
||||||
queryFn: () => jobsService.getJobHistory(200),
|
queryFn: () => jobsService.getJobHistory(200),
|
||||||
refetchInterval: autoRefresh ? 5000 : false,
|
refetchInterval: (query) => (autoRefresh && !query.state.error ? 5000 : false),
|
||||||
staleTime: 2000,
|
staleTime: 2000,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const { data: stats, refetch: refetchStats } = statsQ;
|
||||||
|
const { data: history, refetch: refetchHistory } = historyQ;
|
||||||
|
const loadError = statsQ.error ?? historyQ.error;
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (selectedTab === 'overview') refetchStats();
|
if (selectedTab === 'overview') refetchStats();
|
||||||
else refetchHistory();
|
else refetchHistory();
|
||||||
@@ -415,7 +432,27 @@ export default function JobsPage() {
|
|||||||
|
|
||||||
{/* ── Content ── */}
|
{/* ── Content ── */}
|
||||||
<div className="flex-1 overflow-y-auto p-4 space-y-4">
|
<div className="flex-1 overflow-y-auto p-4 space-y-4">
|
||||||
|
{/* ── Error banner ── */}
|
||||||
|
{loadError && (
|
||||||
|
<div className="flex items-start gap-2 rounded-md border border-red-500/40 bg-red-500/10 px-3 py-2 text-sm text-red-300">
|
||||||
|
<AlertCircle className="w-4 h-4 mt-0.5 shrink-0" />
|
||||||
|
<div className="space-y-1">
|
||||||
|
<p className="font-medium">Could not load job data</p>
|
||||||
|
<p className="text-xs opacity-80">{describeError(loadError)}</p>
|
||||||
|
<button
|
||||||
|
onClick={() => { refetchStats(); refetchHistory(); }}
|
||||||
|
className="text-xs underline hover:no-underline"
|
||||||
|
>
|
||||||
|
Retry
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
|
|
||||||
{/* ── Overview tab ── */}
|
{/* ── Overview tab ── */}
|
||||||
|
{selectedTab === 'overview' && !stats && !loadError && (
|
||||||
|
<div className="text-center py-16 text-muted text-sm">Loading queue stats…</div>
|
||||||
|
)}
|
||||||
{selectedTab === 'overview' && stats && (
|
{selectedTab === 'overview' && stats && (
|
||||||
<div className="grid grid-cols-2 md:grid-cols-3 lg:grid-cols-6 gap-4">
|
<div className="grid grid-cols-2 md:grid-cols-3 lg:grid-cols-6 gap-4">
|
||||||
<StatCard icon={Clock} label="Waiting" value={stats.waiting} color="#eab308" />
|
<StatCard icon={Clock} label="Waiting" value={stats.waiting} color="#eab308" />
|
||||||
|
|||||||
Reference in New Issue
Block a user