web-player: fix open redirect via returnTo, require SESSION_SECRET and validate API inputs

This commit is contained in:
2026-09-24 04:58:08 +02:00
parent b2f450dcf7
commit 3e546d4a1e
12 changed files with 64 additions and 13 deletions
+3 -2
View File
@@ -11,8 +11,9 @@ ZITADEL_CLIENT_SECRET=your-web-app-client-secret
AUTH_REDIRECT_URI=http://localhost:5173/auth/callback AUTH_REDIRECT_URI=http://localhost:5173/auth/callback
AUTH_POST_LOGOUT_URI=http://localhost:5173/ AUTH_POST_LOGOUT_URI=http://localhost:5173/
# Secret used to derive the session cookie encryption key (any long random string) # Secret used to derive the session cookie encryption key. Required, at least
SESSION_SECRET=change-me-to-a-long-random-string # 32 characters (e.g. `openssl rand -base64 48`).
SESSION_SECRET=change-me-to-a-long-random-string-of-32-chars-or-more
# Backend API base URL # Backend API base URL
API_BASE_URL=http://localhost:5111 API_BASE_URL=http://localhost:5111
+1
View File
@@ -1,4 +1,5 @@
export const EXPLORE_PAGE_SIZE = 27; export const EXPLORE_PAGE_SIZE = 27;
export const MAX_PAGE_SIZE = 100;
export const SEARCH_DEBOUNCE_MS = 450; export const SEARCH_DEBOUNCE_MS = 450;
export const SEARCH_MIN_LENGTH = 2; export const SEARCH_MIN_LENGTH = 2;
export const INFINITE_SCROLL_ROOT_MARGIN = '300px'; export const INFINITE_SCROLL_ROOT_MARGIN = '300px';
+21
View File
@@ -0,0 +1,21 @@
import { describe, expect, it } from 'vitest';
import { safeReturnTo } from './auth';
describe('safeReturnTo', () => {
it('keeps local paths', () => {
expect(safeReturnTo('/')).toBe('/');
expect(safeReturnTo('/playlists/1?tab=x#top')).toBe('/playlists/1?tab=x#top');
});
it('falls back to / for missing values', () => {
expect(safeReturnTo(null)).toBe('/');
expect(safeReturnTo(undefined)).toBe('/');
expect(safeReturnTo('')).toBe('/');
});
it('rejects absolute and protocol-relative URLs', () => {
expect(safeReturnTo('https://example.com')).toBe('/');
expect(safeReturnTo('//example.com')).toBe('/');
expect(safeReturnTo('/\\example.com')).toBe('/');
expect(safeReturnTo('javascript:alert(1)')).toBe('/');
expect(safeReturnTo('relative/path')).toBe('/');
});
});
+7
View File
@@ -18,6 +18,13 @@ export interface Session extends SessionUser {
expiresAt: number; expiresAt: number;
} }
export function safeReturnTo(value: string | null | undefined): string {
if (!value || !value.startsWith('/') || value.startsWith('//') || value.startsWith('/\\')) {
return '/';
}
return value;
}
let configPromise: Promise<client.Configuration> | undefined; let configPromise: Promise<client.Configuration> | undefined;
export function getOidcConfig(): Promise<client.Configuration> { export function getOidcConfig(): Promise<client.Configuration> {
configPromise ??= (async () => { configPromise ??= (async () => {
+8 -1
View File
@@ -1,6 +1,7 @@
import { env } from '$env/dynamic/private'; import { env } from '$env/dynamic/private';
const DAY_SECONDS = 60 * 60 * 24; const DAY_SECONDS = 60 * 60 * 24;
const MIN_SESSION_SECRET_LENGTH = 32;
const DEFAULT_OIDC_SCOPE = 'openid profile email offline_access'; const DEFAULT_OIDC_SCOPE = 'openid profile email offline_access';
function positiveInt(value: string | undefined, fallback: number): number { function positiveInt(value: string | undefined, fallback: number): number {
@@ -35,7 +36,13 @@ export const authConfig = {
return required('AUTH_POST_LOGOUT_URI', env.AUTH_POST_LOGOUT_URI); return required('AUTH_POST_LOGOUT_URI', env.AUTH_POST_LOGOUT_URI);
}, },
get sessionSecret() { get sessionSecret() {
return env.SESSION_SECRET; const secret = required('SESSION_SECRET', env.SESSION_SECRET);
if (secret.length < MIN_SESSION_SECRET_LENGTH) {
throw new Error(
`SESSION_SECRET must be at least ${MIN_SESSION_SECRET_LENGTH} characters long.`
);
}
return secret;
}, },
get sessionTtlSeconds() { get sessionTtlSeconds() {
return positiveInt(env.SESSION_TTL_SECONDS, 7 * DAY_SECONDS); return positiveInt(env.SESSION_TTL_SECONDS, 7 * DAY_SECONDS);
+3 -1
View File
@@ -10,7 +10,9 @@ export async function proxyCoverImage(
): Promise<Response> { ): Promise<Response> {
const safeSize = size && ALLOWED_SIZES.has(size) ? size : 'medium'; const safeSize = size && ALLOWED_SIZES.has(size) ? size : 'medium';
const upstream = await fetch(`${apiConfig.baseUrl}/${kind}/${id}/cover?size=${safeSize}`); const upstream = await fetch(
`${apiConfig.baseUrl}/${kind}/${encodeURIComponent(id)}/cover?size=${safeSize}`
);
if (!upstream.ok) { if (!upstream.ok) {
return new Response(null, { status: upstream.status }); return new Response(null, { status: upstream.status });
+6 -2
View File
@@ -5,9 +5,13 @@ import { createApiClient } from '$lib/server/api';
export const POST: RequestHandler = async ({ request, locals, fetch }) => { export const POST: RequestHandler = async ({ request, locals, fetch }) => {
if (!locals.accessToken) return json({ message: 'Inicia sesión.' }, { status: 401 }); if (!locals.accessToken) return json({ message: 'Inicia sesión.' }, { status: 401 });
const body = await request.json(); const payload = await request.json().catch(() => null);
const trackId = payload?.trackId;
if (typeof trackId !== 'string' || trackId === '')
return json({ message: 'Canción inválida.' }, { status: 400 });
const api = createApiClient({ fetch, accessToken: locals.accessToken }); const api = createApiClient({ fetch, accessToken: locals.accessToken });
const result = await api.POST('/likes/toggle', { body }); const result = await api.POST('/likes/toggle', { body: { trackId } });
if (result.error || result.data === undefined) if (result.error || result.data === undefined)
return json({ message: 'No se pudo actualizar Me gusta.' }, { status: 502 }); return json({ message: 'No se pudo actualizar Me gusta.' }, { status: 502 });
@@ -5,7 +5,8 @@ import { createApiClient } from '$lib/server/api';
export const PUT: RequestHandler = async ({ params, request, locals, fetch }) => { export const PUT: RequestHandler = async ({ params, request, locals, fetch }) => {
if (!locals.accessToken) return json({ message: 'Inicia sesión.' }, { status: 401 }); if (!locals.accessToken) return json({ message: 'Inicia sesión.' }, { status: 401 });
const { playedSeconds } = (await request.json()) as { playedSeconds?: number }; const payload = (await request.json().catch(() => null)) as { playedSeconds?: unknown } | null;
const playedSeconds = payload?.playedSeconds;
if (typeof playedSeconds !== 'number' || !Number.isFinite(playedSeconds) || playedSeconds < 0) if (typeof playedSeconds !== 'number' || !Number.isFinite(playedSeconds) || playedSeconds < 0)
return json({ message: 'Tiempo inválido.' }, { status: 400 }); return json({ message: 'Tiempo inválido.' }, { status: 400 });
+5 -2
View File
@@ -1,13 +1,16 @@
import type { RequestHandler } from './$types'; import type { RequestHandler } from './$types';
import { json } from '@sveltejs/kit'; import { json } from '@sveltejs/kit';
import { createApiClient, unwrapOrError } from '$lib/server/api'; import { createApiClient, unwrapOrError } from '$lib/server/api';
import { EXPLORE_PAGE_SIZE } from '$lib/config'; import { EXPLORE_PAGE_SIZE, MAX_PAGE_SIZE } from '$lib/config';
export const GET: RequestHandler = async ({ url, locals, fetch }) => { export const GET: RequestHandler = async ({ url, locals, fetch }) => {
const name = url.searchParams.get('name')?.trim() || undefined; const name = url.searchParams.get('name')?.trim() || undefined;
const genre = url.searchParams.get('genre')?.trim() || undefined; const genre = url.searchParams.get('genre')?.trim() || undefined;
const pageNumber = Math.max(1, Number(url.searchParams.get('pageNumber')) || 1); const pageNumber = Math.max(1, Number(url.searchParams.get('pageNumber')) || 1);
const pageSize = Math.max(1, Number(url.searchParams.get('pageSize')) || EXPLORE_PAGE_SIZE); const pageSize = Math.min(
MAX_PAGE_SIZE,
Math.max(1, Number(url.searchParams.get('pageSize')) || EXPLORE_PAGE_SIZE)
);
const api = createApiClient({ fetch, accessToken: locals.accessToken ?? undefined }); const api = createApiClient({ fetch, accessToken: locals.accessToken ?? undefined });
const result = await api.GET('/tracks', { const result = await api.GET('/tracks', {
+4 -2
View File
@@ -1,9 +1,11 @@
import { redirect } from '@sveltejs/kit'; import { redirect } from '@sveltejs/kit';
import type { PageServerLoad } from './$types'; import type { PageServerLoad } from './$types';
import { safeReturnTo } from '$lib/server/auth';
export const load: PageServerLoad = ({ locals, url }) => { export const load: PageServerLoad = ({ locals, url }) => {
const returnTo = safeReturnTo(url.searchParams.get('returnTo'));
if (locals.user) { if (locals.user) {
redirect(302, url.searchParams.get('returnTo') ?? '/'); redirect(302, returnTo);
} }
return { returnTo: url.searchParams.get('returnTo') ?? '/' }; return { returnTo };
}; };
@@ -5,6 +5,7 @@ import {
getOidcConfig, getOidcConfig,
encodeSession, encodeSession,
toSessionUser, toSessionUser,
safeReturnTo,
writeSessionCookie, writeSessionCookie,
type Session, type Session,
STATE_COOKIE, STATE_COOKIE,
@@ -17,7 +18,7 @@ export const GET: RequestHandler = async ({ url, cookies }) => {
const codeVerifier = cookies.get(VERIFIER_COOKIE); const codeVerifier = cookies.get(VERIFIER_COOKIE);
const state = cookies.get(STATE_COOKIE); const state = cookies.get(STATE_COOKIE);
const nonce = cookies.get(NONCE_COOKIE); const nonce = cookies.get(NONCE_COOKIE);
const returnTo = cookies.get(RETURN_COOKIE) ?? '/'; const returnTo = safeReturnTo(cookies.get(RETURN_COOKIE));
if (!codeVerifier || !state) { if (!codeVerifier || !state) {
error(400, 'Estado de autenticación inválido. Vuelve a iniciar sesión.'); error(400, 'Estado de autenticación inválido. Vuelve a iniciar sesión.');
+2 -1
View File
@@ -3,6 +3,7 @@ import * as client from 'openid-client';
import type { RequestHandler } from './$types'; import type { RequestHandler } from './$types';
import { import {
getOidcConfig, getOidcConfig,
safeReturnTo,
STATE_COOKIE, STATE_COOKIE,
VERIFIER_COOKIE, VERIFIER_COOKIE,
NONCE_COOKIE, NONCE_COOKIE,
@@ -19,7 +20,7 @@ export const GET: RequestHandler = async ({ url, cookies }) => {
const nonce = client.randomNonce(); const nonce = client.randomNonce();
const register = url.searchParams.get('mode') === 'register'; const register = url.searchParams.get('mode') === 'register';
const returnTo = url.searchParams.get('returnTo') ?? '/'; const returnTo = safeReturnTo(url.searchParams.get('returnTo'));
const parameters: Record<string, string> = { const parameters: Record<string, string> = {
redirect_uri: authConfig.redirectUri, redirect_uri: authConfig.redirectUri,