diff --git a/backend/__tests__/routes/adminSettingsRateLimit.test.js b/backend/__tests__/routes/adminSettingsRateLimit.test.js new file mode 100644 index 00000000..bc9dcf2f --- /dev/null +++ b/backend/__tests__/routes/adminSettingsRateLimit.test.js @@ -0,0 +1,123 @@ +/** + * The general API rate limiter's settings, read and written by the admin + * (#1337). + * + * Until now the six rate_limit_* keys had a write route and no screen, and + * the write route used a plain UPDATE — on a fresh install, which has no + * rows, it answered 200 and changed nothing. The settings read did not + * mention the keys at all when they had no row, so the budget in force + * (300 per 15 minutes per IP) was invisible. Pins: the read surfaces the + * defaults, the write creates rows, validation holds, and the limiter picks + * the new values up at once. + */ +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +process.env.NODE_ENV = 'test'; +process.env.TEST_DATABASE_PATH = path.join( + fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-ratelimit-')), 'db.sqlite', +); +process.env.JWT_SECRET = process.env.JWT_SECRET || 'ratelimit-test-secret'; +process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-ratelimit-storage-')); + +const request = require('supertest'); +const express = require('express'); +const cookieParser = require('cookie-parser'); +const { bootCrmDb, seedMinimal, assignAdminRole, mintAdminToken } = require('../integration/helpers/crmDb'); +const { clearPermissionCache } = require('../../src/middleware/permissions'); +const rateLimitService = require('../../src/services/rateLimitService'); +const { MemoryStore } = require('express-rate-limit'); + +describe('admin rate limiter settings', () => { + let db; let cleanup; let app; let tok; let general; + const auth = (req) => req.set('Authorization', `Bearer ${tok}`); + const rows = () => db('app_settings').where('setting_key', 'like', 'rate_limit_%').orderBy('setting_key'); + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + const { adminId } = await seedMinimal(db); + await assignAdminRole(db, adminId, 'super_admin'); + tok = mintAdminToken(adminId); + await db('app_settings').where('setting_key', 'like', 'rate_limit_%').delete(); + clearPermissionCache(); + app = express(); + app.use(express.json()); + app.use(cookieParser()); + app.use('/api/admin/settings', require('../../src/routes/adminSettings')); + }, 120000); + afterAll(async () => { if (cleanup) await cleanup(); }); + + it('surfaces the code defaults in the settings read when no row exists', async () => { + expect(await rows()).toHaveLength(0); + const res = await auth(request(app).get('/api/admin/settings')); + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + rate_limit_enabled: true, rate_limit_window_minutes: 15, rate_limit_max_requests: 300, + rate_limit_auth_max_requests: 5, rate_limit_skip_authenticated: true, rate_limit_public_endpoints_only: false, + }); + // and only when asked for, with a key filter + const filtered = await auth(request(app).get('/api/admin/settings?keys=rate_limit_max_requests,general_site_url')); + expect(filtered.body.rate_limit_max_requests).toBe(300); + expect(filtered.body).not.toHaveProperty('rate_limit_window_minutes'); + }); + + it('creates the rows on a fresh install and the limiter sees the change at once', async () => { + const before = await rateLimitService.getRateLimitSettings(); + expect(before.maxRequests).toBe(300); + const res = await auth(request(app).put('/api/admin/settings/security/rate-limit')).send({ + rate_limit_enabled: true, rate_limit_window_minutes: 10, rate_limit_max_requests: 5000, + rate_limit_auth_max_requests: 8, rate_limit_skip_authenticated: true, rate_limit_public_endpoints_only: false, + }); + expect(res.status).toBe(200); + const stored = await rows(); + expect(stored.map((r) => r.setting_key)).toEqual([ + 'rate_limit_auth_max_requests', 'rate_limit_enabled', 'rate_limit_max_requests', + 'rate_limit_public_endpoints_only', 'rate_limit_skip_authenticated', 'rate_limit_window_minutes', + ]); + expect(stored.every((r) => r.setting_type === 'security')).toBe(true); + const read = await auth(request(app).get('/api/admin/settings')); + expect(read.body.rate_limit_max_requests).toBe(5000); + expect(read.body.rate_limit_window_minutes).toBe(10); + // The route clears the limiter's 60-second cache, so the new budget applies now. + // The window is fixed per limiter instance, so the route rebuilds them too. + expect(rateLimitService.getGeneralLimiter()).toEqual(expect.any(Function)); + expect(rateLimitService.getAuthLimiter()).toEqual(expect.any(Function)); + general = rateLimitService.getGeneralLimiter(); + const after = await rateLimitService.getRateLimitSettings(); + expect(after.maxRequests).toBe(5000); + expect(after.windowMinutes).toBe(10); + expect(after.authMaxRequests).toBe(8); + }); + + it('updates existing rows rather than duplicating them', async () => { + // A rebuild must not leak the previous stores' cleanup intervals. + const shutdown = jest.spyOn(MemoryStore.prototype, 'shutdown'); + await auth(request(app).put('/api/admin/settings/security/rate-limit')).send({ + rate_limit_enabled: false, rate_limit_window_minutes: 15, rate_limit_max_requests: 300, + rate_limit_auth_max_requests: 5, rate_limit_skip_authenticated: false, rate_limit_public_endpoints_only: true, + }); + expect(await rows()).toHaveLength(6); + const after = await rateLimitService.getRateLimitSettings(); + expect(after).toMatchObject({ enabled: false, maxRequests: 300, skipAuthenticated: false, publicEndpointsOnly: true }); + // and every save hands the gates a fresh instance, shutting the old stores down + expect(rateLimitService.getGeneralLimiter()).not.toBe(general); + expect(shutdown).toHaveBeenCalledTimes(2); + shutdown.mockRestore(); + }); + + it('rejects values outside the documented ranges', async () => { + for (const bad of [ + { rate_limit_window_minutes: 0 }, { rate_limit_window_minutes: 61 }, + { rate_limit_max_requests: 9 }, { rate_limit_max_requests: 10001 }, + { rate_limit_auth_max_requests: 0 }, { rate_limit_enabled: 'yes' }, + ]) { + const res = await auth(request(app).put('/api/admin/settings/security/rate-limit')).send({ + rate_limit_enabled: true, rate_limit_window_minutes: 15, rate_limit_max_requests: 300, + rate_limit_auth_max_requests: 5, rate_limit_skip_authenticated: true, rate_limit_public_endpoints_only: false, + ...bad, + }); + expect(res.status).toBe(400); + } + }); +}); diff --git a/backend/docs/SECURITY_LOGGING.md b/backend/docs/SECURITY_LOGGING.md index 36ea2312..5b3d6c72 100644 --- a/backend/docs/SECURITY_LOGGING.md +++ b/backend/docs/SECURITY_LOGGING.md @@ -87,7 +87,7 @@ When rate limits are exceeded, the following is logged: ## Configuration Settings -The general limiter reads these keys from `app_settings` (cached for 60 seconds). There is no admin screen for them yet; they are set through `PUT /api/admin/settings/security/rate-limit` (all six fields required) or directly in the table, JSON-encoded. The defaults below are what applies when a key has no row — a fresh install has none. Note that the route only updates rows that already exist: on a fresh install it answers 200 without writing anything, so insert the six rows directly first. +The general limiter reads these keys from `app_settings` (cached for 60 seconds). They are edited in the admin panel under Settings → Security (the API rate limiter card), which saves through `PUT /api/admin/settings/security/rate-limit` (all six fields required). The route upserts, so a fresh install needs no rows first, and the live limiters are rebuilt on save, so a new window applies without a restart. The defaults below are what applies when a key has no row; the settings read fills them in, so the form shows the budget in force. | Setting | Default | Range | Description | |---------|---------|-------|-------------| diff --git a/backend/server.js b/backend/server.js index 02a7c715..921e9b33 100644 --- a/backend/server.js +++ b/backend/server.js @@ -88,7 +88,7 @@ const backgroundProcessor = require('./src/services/backgroundProcessor'); const { maintenanceMiddleware } = require('./src/middleware/maintenance'); const { sessionTimeoutMiddleware } = require('./src/middleware/sessionTimeout'); const { errorHandler, notFoundHandler } = require('./src/middleware/errorHandler'); -const { createRateLimiter, createAuthRateLimiter } = require('./src/services/rateLimitService'); +const rateLimitService = require('./src/services/rateLimitService'); const { createApiRateLimitGate } = require('./src/middleware/apiRateLimitGate'); const { createAuthRateLimitGate } = require('./src/middleware/authRateLimitGate'); const { getPublicSitePayload } = require('./src/services/publicSiteService'); @@ -293,8 +293,6 @@ app.get(['/health', '/api/health'], async (req, res) => { }); // Initialize rate limiters (they will be created dynamically) -let generalRateLimiter; -let authRateLimiter; function composeInlineStyles(payload) { const { branding } = payload; @@ -487,8 +485,10 @@ async function handlePublicSiteRequest(req, res, next) { // Function to initialize rate limiters async function initializeRateLimiters() { - generalRateLimiter = await createRateLimiter(); - authRateLimiter = await createAuthRateLimiter(); + // The instances live in rateLimitService so the settings route can + // rebuild them when the window changes (#1337); the gates below read + // them per request through the service's getters. + await rateLimitService.initializeRateLimiters(); // Neither limiter is registered here — an app.use() at this point runs after // the routers, the /api 404 handler and the error handler are already on the @@ -511,13 +511,13 @@ async function initializeRateLimiters() { // must stay unmounted (no path argument) so req.path keeps its /api prefix. // /health and /api/health are mounted above this point and so are never // counted, which matters because monitors poll them every couple of seconds. -app.use(createApiRateLimitGate(() => generalRateLimiter)); +app.use(createApiRateLimitGate(rateLimitService.getGeneralLimiter)); // Per-IP limit for credential-verification endpoints only, on its own bucket. // Registered after the general gate so that an IP already over the /api budget // is rejected there first; see authRateLimitGate for the exact endpoint table // and why it must stay unmounted. -app.use(createAuthRateLimitGate(() => authRateLimiter)); +app.use(createAuthRateLimitGate(rateLimitService.getAuthLimiter)); // Body limits. 50mb is only needed by the authenticated admin and API-token // surfaces (restore manifests, CMS and email templates, bulk operations); diff --git a/backend/src/routes/adminSettings.js b/backend/src/routes/adminSettings.js index ded78ed7..bc405dcf 100644 --- a/backend/src/routes/adminSettings.js +++ b/backend/src/routes/adminSettings.js @@ -10,7 +10,7 @@ const { formatBoolean } = require('../utils/dbCompat'); const { adminAuth } = require('../middleware/auth'); const { requirePermission, userHasAnyPermission } = require('../middleware/permissions'); const { clearMaintenanceCache } = require('../middleware/maintenance'); -const { clearSettingsCache } = require('../services/rateLimitService'); +const { clearSettingsCache, initializeRateLimiters, RATE_LIMIT_DEFAULTS } = require('../services/rateLimitService'); const { DEFAULT_PUBLIC_SITE_HTML, DEFAULT_PUBLIC_SITE_CSS, @@ -282,6 +282,14 @@ router.get('/', adminAuth, requirePermission('settings.view'), async (req, res) settingsObject.analytics_rybbit_api_key = '••••••••'; } + // The general API rate limiter falls back to code defaults when a key has + // no row, which is every fresh install. Surface those so the Security tab + // shows the budget actually in force instead of an empty field (#1337). + for (const [key, value] of Object.entries(RATE_LIMIT_DEFAULTS)) { + if (settingsObject[key] === undefined && (!keysFilter || keysFilter.includes(key))) { + settingsObject[key] = value; + } + } res.json(settingsObject); } catch (error) { errorResponse(res, error, 500, 'Failed to fetch settings'); @@ -2067,10 +2075,19 @@ router.put('/security/rate-limit', adminAuth, requirePermission('settings.securi { key: 'rate_limit_public_endpoints_only', value: rate_limit_public_endpoints_only } ]; + // Upsert, not update: a fresh install has no rate_limit_* rows, and a + // plain update matched nothing there — the route answered 200 and + // changed nothing (#1337). for (const { key, value } of settings) { await db('app_settings') - .where('setting_key', key) - .update({ + .insert({ + setting_key: key, + setting_value: JSON.stringify(value), + setting_type: 'security', + updated_at: new Date() + }) + .onConflict('setting_key') + .merge({ setting_value: JSON.stringify(value), updated_at: new Date() }); @@ -2078,6 +2095,10 @@ router.put('/security/rate-limit', adminAuth, requirePermission('settings.securi // Clear the rate limit settings cache to apply changes immediately clearSettingsCache(); + // max and skip re-read the settings per request, the window is fixed + // per limiter instance: rebuild so a changed window applies now rather + // than after a restart (#1337). Counters start fresh. + await initializeRateLimiters(); // Log activity await logActivity('settings_updated', diff --git a/backend/src/services/rateLimitService.js b/backend/src/services/rateLimitService.js index e00472bd..c66b4a80 100644 --- a/backend/src/services/rateLimitService.js +++ b/backend/src/services/rateLimitService.js @@ -1,9 +1,23 @@ const rateLimit = require('express-rate-limit'); +const { MemoryStore } = require('express-rate-limit'); const jwt = require('jsonwebtoken'); const { db } = require('../database/db'); const logger = require('../utils/logger'); const { getAdminTokenFromRequest, getGalleryTokenFromRequest } = require('../utils/tokenUtils'); +// What applies when app_settings has no row for a key — a fresh install has +// none. Keyed by setting name so the admin settings read can surface the +// same values (#1337): the Security tab must show the budget that is in +// force, not an empty field that hides it. +const RATE_LIMIT_DEFAULTS = Object.freeze({ + rate_limit_enabled: true, + rate_limit_window_minutes: 15, + rate_limit_max_requests: 300, + rate_limit_auth_max_requests: 5, + rate_limit_skip_authenticated: true, + rate_limit_public_endpoints_only: false +}); + // Cache for rate limit settings let settingsCache = null; let cacheExpiry = 0; @@ -41,12 +55,12 @@ async function getRateLimitSettings() { // roughly twenty guests per window. An explicit app_settings value still // wins over this fallback. const config = { - enabled: true, - windowMinutes: 15, - maxRequests: 300, - authMaxRequests: 5, - skipAuthenticated: true, - publicEndpointsOnly: false + enabled: RATE_LIMIT_DEFAULTS.rate_limit_enabled, + windowMinutes: RATE_LIMIT_DEFAULTS.rate_limit_window_minutes, + maxRequests: RATE_LIMIT_DEFAULTS.rate_limit_max_requests, + authMaxRequests: RATE_LIMIT_DEFAULTS.rate_limit_auth_max_requests, + skipAuthenticated: RATE_LIMIT_DEFAULTS.rate_limit_skip_authenticated, + publicEndpointsOnly: RATE_LIMIT_DEFAULTS.rate_limit_public_endpoints_only }; settings.forEach(setting => { @@ -211,10 +225,11 @@ function shouldSkipRateLimit(req, config) { /** * Create dynamic rate limiter */ -async function createRateLimiter() { +async function createRateLimiter(store = new MemoryStore()) { const config = await getRateLimitSettings(); return rateLimit({ + store, windowMs: config.windowMinutes * 60 * 1000, max: async (req) => { // Refresh config for each request @@ -278,10 +293,11 @@ async function createRateLimiter() { * makes a 5-per-window per-IP budget safe behind NAT — a room full of guests on * one venue IP who all type the correct gallery password consume nothing. */ -async function createAuthRateLimiter() { +async function createAuthRateLimiter(store = new MemoryStore()) { const config = await getRateLimitSettings(); return rateLimit({ + store, windowMs: config.windowMinutes * 60 * 1000, // Read per request, like the general limiter, so a change to // rate_limit_auth_max_requests in admin Settings takes effect within the @@ -330,7 +346,40 @@ async function createAuthRateLimiter() { }); } +// The live limiter instances. express-rate-limit fixes windowMs when an +// instance is built — max and skip re-read the settings per request, the +// window does not — so a saved window only takes effect on a rebuild. The +// gates in server.js resolve the instance per request through the getters +// below, and the settings route rebuilds after a save (#1337). Rebuilding +// starts fresh counters; on a settings change that is acceptable. +// +// The stores are held explicitly because a MemoryStore runs a cleanup +// interval for as long as it exists: dropping the limiter reference alone +// would leave one more live timer and one more retained store per save. +const current = { general: null, auth: null, stores: [] }; + +async function initializeRateLimiters() { + const stores = [new MemoryStore(), new MemoryStore()]; + const general = await createRateLimiter(stores[0]); + const auth = await createAuthRateLimiter(stores[1]); + const superseded = current.stores; + current.general = general; + current.auth = auth; + current.stores = stores; + for (const store of superseded) { + if (typeof store.shutdown === 'function') store.shutdown(); + } + return current; +} + +const getGeneralLimiter = () => current.general; +const getAuthLimiter = () => current.auth; + module.exports = { + RATE_LIMIT_DEFAULTS, + initializeRateLimiters, + getGeneralLimiter, + getAuthLimiter, getRateLimitSettings, clearSettingsCache, createRateLimiter, diff --git a/frontend/src/features/settings/__tests__/SecurityTab.rateLimit.test.tsx b/frontend/src/features/settings/__tests__/SecurityTab.rateLimit.test.tsx new file mode 100644 index 00000000..907502b3 --- /dev/null +++ b/frontend/src/features/settings/__tests__/SecurityTab.rateLimit.test.tsx @@ -0,0 +1,90 @@ +/** + * The API rate limiter has a screen (#1337). + * + * Its six settings had a backend route and nothing in the UI, so installs + * ran on a 300-per-15-minutes budget nobody could see (issue 1287). The + * Security tab now shows the values in force, edits them through the hook's + * state, and saves them with the tab's Save button. The test i18n renders the + * English strings, so queries use those. + */ +import React from 'react'; +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import { SecurityTab } from '../tabs/SecurityTab'; +import { validateRateLimitSettings, type SecuritySettings, type RateLimitSettings } from '../hooks/useSettingsState'; + +const security: SecuritySettings = { + password_min_length: 8, password_complexity: 'strong', session_timeout_minutes: 60, + max_login_attempts: 5, attempt_window_minutes: 15, lockout_duration_minutes: 30, + enable_recaptcha: false, recaptcha_site_key: '', recaptcha_secret_key: '', +}; +const rateLimit: RateLimitSettings = { + rate_limit_enabled: true, rate_limit_window_minutes: 15, rate_limit_max_requests: 300, + rate_limit_auth_max_requests: 5, rate_limit_skip_authenticated: true, rate_limit_public_endpoints_only: false, +}; + +function mount(overrides: Partial = {}) { + const setRateLimitSettings = vi.fn(); + const mutate = vi.fn(); + render( + + ); + return { setRateLimitSettings, mutate }; +} + +describe('SecurityTab rate limiter card', () => { + it('shows the budget in force, defaults included', () => { + mount(); + expect(screen.getByLabelText('Max requests per window')).toHaveValue(300); + expect(screen.getByLabelText('Window (minutes)')).toHaveValue(15); + expect(screen.getByLabelText('Failed logins per window')).toHaveValue(5); + expect(screen.getByLabelText('Enable the API rate limiter')).toBeChecked(); + expect(screen.getByLabelText(/Do not count authenticated requests/)).toBeChecked(); + expect(screen.getByLabelText(/Count public endpoints only/)).not.toBeChecked(); + expect(screen.getByText(/The unit is the client IP/)).toBeInTheDocument(); + }); + + it('edits go through the hook state as a functional update', () => { + const { setRateLimitSettings } = mount(); + fireEvent.change(screen.getByLabelText('Max requests per window'), { target: { value: '5000' } }); + expect(setRateLimitSettings).toHaveBeenCalledTimes(1); + const updater = setRateLimitSettings.mock.calls[0][0] as (prev: RateLimitSettings) => RateLimitSettings; + expect(updater(rateLimit)).toEqual({ ...rateLimit, rate_limit_max_requests: 5000 }); + + fireEvent.click(screen.getByLabelText(/Count public endpoints only/)); + const toggle = setRateLimitSettings.mock.calls[1][0] as (prev: RateLimitSettings) => RateLimitSettings; + expect(toggle(rateLimit).rate_limit_public_endpoints_only).toBe(true); + }); + + it('the number fields carry the route\'s validation ranges', () => { + mount(); + expect(screen.getByLabelText('Max requests per window')).toHaveAttribute('min', '10'); + expect(screen.getByLabelText('Max requests per window')).toHaveAttribute('max', '10000'); + expect(screen.getByLabelText('Window (minutes)')).toHaveAttribute('max', '60'); + expect(screen.getByLabelText('Failed logins per window')).toHaveAttribute('max', '100'); + }); + + it('the tab\'s Save button saves the limiter along with the rest', () => { + const { mutate } = mount(); + fireEvent.click(screen.getByRole('button', { name: /save security settings/i })); + expect(mutate).toHaveBeenCalledTimes(1); + }); +}); + +describe('rate limiter validation before save', () => { + it('accepts the defaults and the route\'s bounds, rejects outside and non-integers', () => { + expect(validateRateLimitSettings(rateLimit)).toBeNull(); + expect(validateRateLimitSettings({ ...rateLimit, rate_limit_window_minutes: 60, rate_limit_max_requests: 10000, rate_limit_auth_max_requests: 100 })).toBeNull(); + expect(validateRateLimitSettings({ ...rateLimit, rate_limit_window_minutes: 0 })).toBe('rate_limit_window_minutes'); + expect(validateRateLimitSettings({ ...rateLimit, rate_limit_max_requests: 10001 })).toBe('rate_limit_max_requests'); + expect(validateRateLimitSettings({ ...rateLimit, rate_limit_auth_max_requests: 0 })).toBe('rate_limit_auth_max_requests'); + // a cleared number field arrives as NaN + expect(validateRateLimitSettings({ ...rateLimit, rate_limit_max_requests: Number('') })).toBe('rate_limit_max_requests'); + }); +}); diff --git a/frontend/src/features/settings/hooks/useSettingsState.ts b/frontend/src/features/settings/hooks/useSettingsState.ts index 7717c55e..529c3176 100644 --- a/frontend/src/features/settings/hooks/useSettingsState.ts +++ b/frontend/src/features/settings/hooks/useSettingsState.ts @@ -57,6 +57,32 @@ export interface SecuritySettings { recaptcha_secret_key: string; } +/** The general per-IP API rate limiter (#1337). Keys match app_settings. */ +export interface RateLimitSettings { + rate_limit_enabled: boolean; + rate_limit_window_minutes: number; + rate_limit_max_requests: number; + rate_limit_auth_max_requests: number; + rate_limit_skip_authenticated: boolean; + rate_limit_public_endpoints_only: boolean; +} + +/** The ranges the backend route enforces; checked before anything is written. */ +export const RATE_LIMIT_RANGES: Record<'rate_limit_window_minutes' | 'rate_limit_max_requests' | 'rate_limit_auth_max_requests', [number, number]> = { + rate_limit_window_minutes: [1, 60], + rate_limit_max_requests: [10, 10000], + rate_limit_auth_max_requests: [1, 100] +}; + +/** Returns the first out-of-range field, or null when everything is valid. */ +export function validateRateLimitSettings(settings: RateLimitSettings): keyof typeof RATE_LIMIT_RANGES | null { + for (const [key, [min, max]] of Object.entries(RATE_LIMIT_RANGES) as Array<[keyof typeof RATE_LIMIT_RANGES, [number, number]]>) { + const value = settings[key]; + if (!Number.isInteger(value) || value < min || value > max) return key; + } + return null; +} + export type TrackerProvider = 'none' | 'umami' | 'rybbit' | 'custom'; export interface AnalyticsSettings { @@ -167,6 +193,17 @@ export function useSettingsState() { recaptcha_secret_key: '' }); + // Rate limiter state. The fallbacks mirror the backend's defaults, but the + // settings read fills every key, so they only matter before the first load. + const [rateLimitSettings, setRateLimitSettings] = useState({ + rate_limit_enabled: true, + rate_limit_window_minutes: 15, + rate_limit_max_requests: 300, + rate_limit_auth_max_requests: 5, + rate_limit_skip_authenticated: true, + rate_limit_public_endpoints_only: false + }); + // Analytics settings state const [analyticsSettings, setAnalyticsSettings] = useState({ tracker_provider: 'none', @@ -275,6 +312,15 @@ export function useSettingsState() { recaptcha_secret_key: settings.security_recaptcha_secret_key ?? '' }); + setRateLimitSettings({ + rate_limit_enabled: toBoolean(settings.rate_limit_enabled, true), + rate_limit_window_minutes: toNumber(settings.rate_limit_window_minutes, 15), + rate_limit_max_requests: toNumber(settings.rate_limit_max_requests, 300), + rate_limit_auth_max_requests: toNumber(settings.rate_limit_auth_max_requests, 5), + rate_limit_skip_authenticated: toBoolean(settings.rate_limit_skip_authenticated, true), + rate_limit_public_endpoints_only: toBoolean(settings.rate_limit_public_endpoints_only, false) + }); + // Tracker provider: prefer explicit setting; fall back to legacy // umami_enabled flag for installs that haven't picked yet (#663). const explicitProvider = settings.analytics_tracker_provider; @@ -401,14 +447,22 @@ export function useSettingsState() { Object.entries(securitySettings).forEach(([key, value]) => { settingsData[`security_${key}`] = value; }); - return settingsService.updateSettings(settingsData); + // Nothing is written until the limiter values pass the same ranges the + // route enforces, and the limiter goes first: a 400 from its route + // would otherwise land after the password/session settings were + // already persisted, a half-applied save reported as failed (#1337). + if (validateRateLimitSettings(rateLimitSettings)) throw new Error('RATE_LIMIT_INVALID'); + await settingsService.updateRateLimit(rateLimitSettings); + await settingsService.updateSettings(settingsData); }, onSuccess: () => { toast.success(t('toast.settingsSaved')); queryClient.invalidateQueries({ queryKey: ['admin-settings'] }); }, - onError: () => { - toast.error(t('toast.saveError')); + onError: (error: unknown) => { + toast.error(t(error instanceof Error && error.message === 'RATE_LIMIT_INVALID' + ? 'settings.security.rateLimitInvalid' + : 'toast.saveError')); } }); @@ -653,6 +707,8 @@ export function useSettingsState() { setGeneralSettings, securitySettings, setSecuritySettings, + rateLimitSettings, + setRateLimitSettings, analyticsSettings, setAnalyticsSettings, eventSettings, diff --git a/frontend/src/features/settings/tabs/SecurityTab.tsx b/frontend/src/features/settings/tabs/SecurityTab.tsx index cb338f13..5b88e904 100644 --- a/frontend/src/features/settings/tabs/SecurityTab.tsx +++ b/frontend/src/features/settings/tabs/SecurityTab.tsx @@ -2,11 +2,13 @@ import React from 'react'; import { Save, Key, AlertCircle, ShieldCheck } from 'lucide-react'; import { Button, Card, Input } from '../../../components/common'; import { useTranslation } from 'react-i18next'; -import type { SecuritySettings } from '../hooks/useSettingsState'; +import type { SecuritySettings, RateLimitSettings } from '../hooks/useSettingsState'; interface SecurityTabProps { securitySettings: SecuritySettings; setSecuritySettings: React.Dispatch>; + rateLimitSettings: RateLimitSettings; + setRateLimitSettings: React.Dispatch>; saveSecurityMutation: { mutate: () => void; isPending: boolean; @@ -16,9 +18,29 @@ interface SecurityTabProps { export const SecurityTab: React.FC = ({ securitySettings, setSecuritySettings, + rateLimitSettings, + setRateLimitSettings, saveSecurityMutation, }) => { const { t } = useTranslation(); + const setRateLimit = (key: K, value: RateLimitSettings[K]) => + setRateLimitSettings((prev) => ({ ...prev, [key]: value })); + const numberField = (key: keyof RateLimitSettings, min: number, max: number, labelKey: string, helpKey: string) => ( +
+ + setRateLimit(key, Number(e.target.value) as RateLimitSettings[typeof key])} + aria-label={t(`settings.security.${labelKey}`)} + /> +

{t(`settings.security.${helpKey}`)}

+
+ ); return (
@@ -136,6 +158,62 @@ export const SecurityTab: React.FC = ({
+ {/* General API rate limiter (#1337). These keys had a backend route and + no screen, so installs ran on a budget nobody could see. */} + +

{t('settings.security.rateLimitTitle')}

+

{t('settings.security.rateLimitIntro')}

+ +
+ + +
+ {numberField('rate_limit_window_minutes', 1, 60, 'rateLimitWindowMinutes', 'rateLimitWindowMinutesHelp')} + {numberField('rate_limit_max_requests', 10, 10000, 'rateLimitMaxRequests', 'rateLimitMaxRequestsHelp')} + {numberField('rate_limit_auth_max_requests', 1, 100, 'rateLimitAuthMaxRequests', 'rateLimitAuthMaxRequestsHelp')} +
+ + + + + +
+ +

{t('settings.security.rateLimitNatNote')}

+
+
+ +

{t('settings.security.recaptchaSettings')}

diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index 399a5108..3b823905 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -2325,6 +2325,21 @@ "secretKey": "Geheimer Schlüssel", "recaptchaHelp": "Holen Sie sich Ihre reCAPTCHA-Schlüssel von", "saveSecuritySettings": "Sicherheitseinstellungen speichern", + "rateLimitTitle": "API-Ratenbegrenzung", + "rateLimitIntro": "Begrenzt, wie viele API-Anfragen eine Client-IP pro Zeitfenster stellen darf. Wer das Budget überschreitet, bekommt für den Rest des Fensters 429-Antworten. Für Besucher sieht das aus wie eine Galerie, die nicht mehr lädt, nicht wie ein Fehler.", + "rateLimitEnabled": "API-Ratenbegrenzung aktivieren", + "rateLimitWindowMinutes": "Zeitfenster (Minuten)", + "rateLimitWindowMinutesHelp": "Gleitendes Fenster pro Client-IP. 1 bis 60.", + "rateLimitMaxRequests": "Max. Anfragen pro Fenster", + "rateLimitMaxRequestsHelp": "Budget für API-Anfragen, die nicht ausgenommen sind. Standard 300. 10 bis 10000.", + "rateLimitAuthMaxRequests": "Fehlgeschlagene Anmeldungen pro Fenster", + "rateLimitAuthMaxRequestsHelp": "Eigenes Budget für Admin-Anmeldung und Galerie-Passwortprüfung; nur Fehlversuche zählen. Standard 5. 1 bis 100.", + "rateLimitSkipAuthenticated": "Authentifizierte Anfragen nicht zählen", + "rateLimitSkipAuthenticatedHelp": "Admin-Sitzungen sind ausgenommen, ebenso die eigenen Bildanfragen eines angemeldeten Galerie-Besuchers (Vorschaubilder, Vorschauen, Fotos).", + "rateLimitPublicOnly": "Nur öffentliche Endpunkte zählen", + "rateLimitPublicOnlyHelp": "Wenn aktiv, zählen nur Anfragen an /api/public und /api/gallery.", + "rateLimitNatNote": "Die Einheit ist die Client-IP. Ein Büro, ein Haushalt oder das WLAN einer Location teilen sich eine Adresse und damit ein Budget. Hinter einem Reverse-Proxy stimmt die Client-IP nur, wenn TRUST_PROXY den Proxy abdeckt; sonst teilen sich alle Besucher das Budget des Proxys. Abgewiesene Anfragen stehen als „Rate limit exceeded“ im Backend-Log.", + "rateLimitInvalid": "Werte der Ratenbegrenzung außerhalb des Bereichs: Zeitfenster 1–60 Minuten, Anfragen 10–10000, fehlgeschlagene Anmeldungen 1–100. Es wurde nichts gespeichert.", "twoFactorTitle": "Zwei-Faktor-Authentifizierung", "twoFactorNote": "Die Zwei-Faktor-Authentifizierung wird jetzt pro Admin unter Einstellungen → Allgemein → Admin-Konto verwaltet. Jeder Admin aktiviert sie für seine eigene Anmeldung." }, diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 3449b77b..3db9fcad 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -1823,6 +1823,21 @@ "secretKey": "Secret Key", "recaptchaHelp": "Get your reCAPTCHA keys from", "saveSecuritySettings": "Save Security Settings", + "rateLimitTitle": "API rate limiting", + "rateLimitIntro": "Caps how many API requests one client IP may make per window. A visitor over the budget gets 429 responses for the rest of the window, which shows up as a gallery that stops loading, not as an error.", + "rateLimitEnabled": "Enable the API rate limiter", + "rateLimitWindowMinutes": "Window (minutes)", + "rateLimitWindowMinutesHelp": "Sliding window per client IP. 1 to 60.", + "rateLimitMaxRequests": "Max requests per window", + "rateLimitMaxRequestsHelp": "Budget for API requests that are not exempt. Default 300. 10 to 10000.", + "rateLimitAuthMaxRequests": "Failed logins per window", + "rateLimitAuthMaxRequestsHelp": "Separate budget for admin login and gallery password checks; only failed attempts count. Default 5. 1 to 100.", + "rateLimitSkipAuthenticated": "Do not count authenticated requests", + "rateLimitSkipAuthenticatedHelp": "Admin sessions are exempt, and so are a signed-in gallery viewer's own image requests (thumbnails, previews, photos).", + "rateLimitPublicOnly": "Count public endpoints only", + "rateLimitPublicOnlyHelp": "When on, only /api/public and /api/gallery requests count.", + "rateLimitNatNote": "The unit is the client IP. An office, a household or a venue's Wi-Fi share one address and therefore one budget. Behind a reverse proxy the client IP is only correct if TRUST_PROXY covers the proxy; otherwise every visitor shares the proxy's budget. Rejected requests are logged as \"Rate limit exceeded\" in the backend log.", + "rateLimitInvalid": "Rate limiter values are out of range: window 1–60 minutes, requests 10–10000, failed logins 1–100. Nothing was saved.", "twoFactorTitle": "Two-factor authentication", "twoFactorNote": "Two-factor authentication is now managed per admin from Settings → General → Admin Account. Each admin enables it for their own login." }, diff --git a/frontend/src/pages/admin/SettingsPage.tsx b/frontend/src/pages/admin/SettingsPage.tsx index 9ad1a225..9c7261a3 100644 --- a/frontend/src/pages/admin/SettingsPage.tsx +++ b/frontend/src/pages/admin/SettingsPage.tsx @@ -209,6 +209,8 @@ export const SettingsPage: React.FC = () => { generalSettings, setGeneralSettings, securitySettings, + rateLimitSettings, + setRateLimitSettings, setSecuritySettings, analyticsSettings, setAnalyticsSettings, @@ -571,6 +573,8 @@ export const SettingsPage: React.FC = () => { )} diff --git a/frontend/src/services/settings.service.ts b/frontend/src/services/settings.service.ts index 9d4b8c18..1ae7bd4d 100644 --- a/frontend/src/services/settings.service.ts +++ b/frontend/src/services/settings.service.ts @@ -312,6 +312,18 @@ export const settingsService = { await api.put('/admin/settings/theme', settings); }, + // General API rate limiter (#1337). Its own route validates ranges and + // clears the limiter's settings cache, so changes apply at once. + async updateRateLimit(settings: { + rate_limit_enabled: boolean; + rate_limit_window_minutes: number; + rate_limit_max_requests: number; + rate_limit_auth_max_requests: number; + rate_limit_skip_authenticated: boolean; + rate_limit_public_endpoints_only: boolean; + }): Promise { + await api.put('/admin/settings/security/rate-limit', settings); + }, // Update multiple settings at once async updateSettings(settings: Record): Promise { // Determine the endpoint based on setting keys