diff --git a/backend/src/config/environment.ts b/backend/src/config/environment.ts index 53de72a..b820eea 100644 --- a/backend/src/config/environment.ts +++ b/backend/src/config/environment.ts @@ -55,7 +55,7 @@ const environment: EnvironmentConfig = { password: process.env.DB_PASSWORD || 'dev_password', }, jwt: { - secret: process.env.JWT_SECRET || 'your-secret-key-change-in-production', + secret: process.env.JWT_SECRET || '', expiresIn: process.env.JWT_EXPIRES_IN || '24h', }, cors: { @@ -83,4 +83,31 @@ const environment: EnvironmentConfig = { }, }; +function validateEnvironment(env: EnvironmentConfig): void { + const secret = env.jwt.secret; + + if (!secret) { + throw new Error( + 'FATAL: JWT_SECRET is not set. ' + + 'Set a strong, random secret of at least 32 characters before starting the server.' + ); + } + + if (secret === 'your-secret-key-change-in-production') { + throw new Error( + 'FATAL: JWT_SECRET is still set to the known weak default value. ' + + 'Replace it with a strong, random secret of at least 32 characters.' + ); + } + + if (secret.length < 32) { + throw new Error( + `FATAL: JWT_SECRET is too short (${secret.length} characters). ` + + 'A minimum of 32 characters is required.' + ); + } +} + +validateEnvironment(environment); + export default environment; diff --git a/backend/src/controllers/bookstack.controller.ts b/backend/src/controllers/bookstack.controller.ts index 9d2c9b7..5e8893a 100644 --- a/backend/src/controllers/bookstack.controller.ts +++ b/backend/src/controllers/bookstack.controller.ts @@ -28,6 +28,10 @@ class BookStackController { res.status(400).json({ success: false, message: 'Suchbegriff fehlt' }); return; } + if (query.trim().length > 500) { + res.status(400).json({ success: false, message: 'Suchanfrage zu lang' }); + return; + } try { const results = await bookstackService.searchPages(query.trim()); res.status(200).json({ success: true, data: results, configured: true }); diff --git a/backend/src/middleware/error.middleware.ts b/backend/src/middleware/error.middleware.ts index 5dc693c..427af95 100644 --- a/backend/src/middleware/error.middleware.ts +++ b/backend/src/middleware/error.middleware.ts @@ -46,9 +46,7 @@ export const errorHandler = ( res.status(500).json({ status: 'error', - message: process.env.NODE_ENV === 'production' - ? 'Internal server error' - : err.message, + message: 'An internal error occurred', }); }; diff --git a/backend/src/middleware/rbac.middleware.ts b/backend/src/middleware/rbac.middleware.ts index fc632fb..fdbc425 100644 --- a/backend/src/middleware/rbac.middleware.ts +++ b/backend/src/middleware/rbac.middleware.ts @@ -124,7 +124,7 @@ export function requirePermission(permission: string) { res.status(403).json({ success: false, - message: `Keine Berechtigung: ${permission}`, + message: 'Keine Berechtigung', }); return; } diff --git a/backend/src/routes/member.routes.ts b/backend/src/routes/member.routes.ts index 9553811..7f62223 100644 --- a/backend/src/routes/member.routes.ts +++ b/backend/src/routes/member.routes.ts @@ -110,14 +110,29 @@ router.post( memberController.createMemberProfile.bind(memberController) ); +/** + * Inline middleware for PATCH /:userId. + * Enforces that the caller is either the profile owner OR holds members:write. + * This is the route-level IDOR guard; the controller still applies the + * correct Zod schema (full vs. limited fields) based on role. + */ +const requireOwnerOrWrite = (req: Request, res: Response, next: NextFunction): void => { + const isOwner = req.user?.id === req.params.userId; + if (isOwner) { + next(); + return; + } + // Not the owner — must have members:write permission + requirePermission('members:write')(req, res, next); +}; + /** * PATCH /:userId — open to both privileged users AND the profile owner. - * The controller itself enforces the correct Zod schema (full vs. limited) - * based on the caller's role. + * Route-level guard rejects all other callers before the controller runs. */ router.patch( '/:userId', - // No requirePermission here — controller handles own-profile vs. write-role logic + requireOwnerOrWrite, memberController.updateMember.bind(memberController) ); diff --git a/backend/src/services/bookstack.service.ts b/backend/src/services/bookstack.service.ts index 5fdc693..b2ba440 100644 --- a/backend/src/services/bookstack.service.ts +++ b/backend/src/services/bookstack.service.ts @@ -32,6 +32,47 @@ export interface BookStackSearchResult { tags: { name: string; value: string; order: number }[]; } +/** + * Validates that a URL is safe to use as an outbound service endpoint. + * Rejects non-http(s) protocols and private/loopback IP ranges to prevent SSRF. + */ +function isValidServiceUrl(raw: string): boolean { + let parsed: URL; + try { + parsed = new URL(raw); + } catch { + return false; + } + + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + return false; + } + + const hostname = parsed.hostname.toLowerCase(); + + // Reject plain loopback / localhost names + if (hostname === 'localhost' || hostname === '::1') { + return false; + } + + // Reject numeric IPv4 private / loopback / link-local ranges + const ipv4Parts = hostname.split('.'); + if (ipv4Parts.length === 4) { + const [a, b] = ipv4Parts.map(Number); + if ( + a === 127 || // 127.0.0.0/8 loopback + a === 10 || // 10.0.0.0/8 private + (a === 172 && b >= 16 && b <= 31) || // 172.16.0.0/12 private + (a === 192 && b === 168) || // 192.168.0.0/16 private + (a === 169 && b === 254) // 169.254.0.0/16 link-local + ) { + return false; + } + } + + return true; +} + function buildHeaders(): Record { const { bookstack } = environment; return { @@ -42,8 +83,8 @@ function buildHeaders(): Record { async function getRecentPages(): Promise { const { bookstack } = environment; - if (!bookstack.url) { - throw new Error('BOOKSTACK_URL is not configured'); + if (!bookstack.url || !isValidServiceUrl(bookstack.url)) { + throw new Error('BOOKSTACK_URL is not configured or is not a valid service URL'); } try { @@ -73,8 +114,8 @@ async function getRecentPages(): Promise { async function searchPages(query: string): Promise { const { bookstack } = environment; - if (!bookstack.url) { - throw new Error('BOOKSTACK_URL is not configured'); + if (!bookstack.url || !isValidServiceUrl(bookstack.url)) { + throw new Error('BOOKSTACK_URL is not configured or is not a valid service URL'); } try { diff --git a/backend/src/services/nextcloud.service.ts b/backend/src/services/nextcloud.service.ts index c89428c..58aaefc 100644 --- a/backend/src/services/nextcloud.service.ts +++ b/backend/src/services/nextcloud.service.ts @@ -34,10 +34,51 @@ interface LoginFlowCredentials { appPassword: string; } +/** + * Validates that a URL is safe to use as an outbound service endpoint. + * Rejects non-http(s) protocols and private/loopback IP ranges to prevent SSRF. + */ +function isValidServiceUrl(raw: string): boolean { + let parsed: URL; + try { + parsed = new URL(raw); + } catch { + return false; + } + + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + return false; + } + + const hostname = parsed.hostname.toLowerCase(); + + // Reject plain loopback / localhost names + if (hostname === 'localhost' || hostname === '::1') { + return false; + } + + // Reject numeric IPv4 private / loopback / link-local ranges + const ipv4Parts = hostname.split('.'); + if (ipv4Parts.length === 4) { + const [a, b] = ipv4Parts.map(Number); + if ( + a === 127 || // 127.0.0.0/8 loopback + a === 10 || // 10.0.0.0/8 private + (a === 172 && b >= 16 && b <= 31) || // 172.16.0.0/12 private + (a === 192 && b === 168) || // 192.168.0.0/16 private + (a === 169 && b === 254) // 169.254.0.0/16 link-local + ) { + return false; + } + } + + return true; +} + async function initiateLoginFlow(): Promise { const baseUrl = environment.nextcloudUrl; - if (!baseUrl) { - throw new Error('NEXTCLOUD_URL is not configured'); + if (!baseUrl || !isValidServiceUrl(baseUrl)) { + throw new Error('NEXTCLOUD_URL is not configured or is not a valid service URL'); } try { @@ -60,6 +101,9 @@ async function initiateLoginFlow(): Promise { } async function pollLoginFlow(pollEndpoint: string, pollToken: string): Promise { + if (!isValidServiceUrl(pollEndpoint)) { + throw new Error('pollEndpoint is not a valid service URL'); + } try { const response = await axios.post(pollEndpoint, `token=${pollToken}`, { headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, @@ -85,8 +129,8 @@ async function pollLoginFlow(pollEndpoint: string, pollToken: string): Promise { const baseUrl = environment.nextcloudUrl; - if (!baseUrl) { - throw new Error('NEXTCLOUD_URL is not configured'); + if (!baseUrl || !isValidServiceUrl(baseUrl)) { + throw new Error('NEXTCLOUD_URL is not configured or is not a valid service URL'); } try { diff --git a/backend/src/services/vikunja.service.ts b/backend/src/services/vikunja.service.ts index 0b6fa91..499937a 100644 --- a/backend/src/services/vikunja.service.ts +++ b/backend/src/services/vikunja.service.ts @@ -16,6 +16,47 @@ export interface VikunjaProject { title: string; } +/** + * Validates that a URL is safe to use as an outbound service endpoint. + * Rejects non-http(s) protocols and private/loopback IP ranges to prevent SSRF. + */ +function isValidServiceUrl(raw: string): boolean { + let parsed: URL; + try { + parsed = new URL(raw); + } catch { + return false; + } + + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + return false; + } + + const hostname = parsed.hostname.toLowerCase(); + + // Reject plain loopback / localhost names + if (hostname === 'localhost' || hostname === '::1') { + return false; + } + + // Reject numeric IPv4 private / loopback / link-local ranges + const ipv4Parts = hostname.split('.'); + if (ipv4Parts.length === 4) { + const [a, b] = ipv4Parts.map(Number); + if ( + a === 127 || // 127.0.0.0/8 loopback + a === 10 || // 10.0.0.0/8 private + (a === 172 && b >= 16 && b <= 31) || // 172.16.0.0/12 private + (a === 192 && b === 168) || // 192.168.0.0/16 private + (a === 169 && b === 254) // 169.254.0.0/16 link-local + ) { + return false; + } + } + + return true; +} + function buildHeaders(): Record { return { 'Authorization': `Bearer ${environment.vikunja.apiToken}`, @@ -25,8 +66,8 @@ function buildHeaders(): Record { async function getMyTasks(): Promise { const { vikunja } = environment; - if (!vikunja.url) { - throw new Error('VIKUNJA_URL is not configured'); + if (!vikunja.url || !isValidServiceUrl(vikunja.url)) { + throw new Error('VIKUNJA_URL is not configured or is not a valid service URL'); } try { @@ -58,8 +99,8 @@ async function getOverdueTasks(): Promise { async function getProjects(): Promise { const { vikunja } = environment; - if (!vikunja.url) { - throw new Error('VIKUNJA_URL is not configured'); + if (!vikunja.url || !isValidServiceUrl(vikunja.url)) { + throw new Error('VIKUNJA_URL is not configured or is not a valid service URL'); } try { @@ -82,8 +123,8 @@ async function getProjects(): Promise { async function createTask(projectId: number, title: string, dueDate?: string): Promise { const { vikunja } = environment; - if (!vikunja.url) { - throw new Error('VIKUNJA_URL is not configured'); + if (!vikunja.url || !isValidServiceUrl(vikunja.url)) { + throw new Error('VIKUNJA_URL is not configured or is not a valid service URL'); } try { diff --git a/frontend/src/components/auth/LoginCallback.tsx b/frontend/src/components/auth/LoginCallback.tsx index 0d38c46..85d86e1 100644 --- a/frontend/src/components/auth/LoginCallback.tsx +++ b/frontend/src/components/auth/LoginCallback.tsx @@ -30,8 +30,14 @@ const LoginCallback: React.FC = () => { try { await login(code); - // Navigate to the originally intended page, falling back to the dashboard - const from = sessionStorage.getItem('auth_redirect_from') || '/dashboard'; + // Navigate to the originally intended page, falling back to the dashboard. + // Validate that the stored path is a safe internal path: must start with '/' + // but must NOT start with '//' (protocol-relative redirect). + const rawFrom = sessionStorage.getItem('auth_redirect_from'); + const from = + rawFrom && rawFrom.startsWith('/') && !rawFrom.startsWith('//') + ? rawFrom + : '/dashboard'; sessionStorage.removeItem('auth_redirect_from'); navigate(from, { replace: true }); } catch (err) { diff --git a/frontend/src/components/dashboard/BookStackRecentWidget.tsx b/frontend/src/components/dashboard/BookStackRecentWidget.tsx index 7abe083..a312a68 100644 --- a/frontend/src/components/dashboard/BookStackRecentWidget.tsx +++ b/frontend/src/components/dashboard/BookStackRecentWidget.tsx @@ -13,13 +13,14 @@ import { formatDistanceToNow } from 'date-fns'; import { de } from 'date-fns/locale'; import { bookstackApi } from '../../services/bookstack'; import type { BookStackPage } from '../../types/bookstack.types'; +import { safeOpenUrl } from '../../utils/safeOpenUrl'; const PageRow: React.FC<{ page: BookStackPage; showDivider: boolean }> = ({ page, showDivider, }) => { const handleClick = () => { - window.open(page.url, '_blank', 'noopener,noreferrer'); + safeOpenUrl(page.url); }; const relativeTime = page.updated_at diff --git a/frontend/src/components/dashboard/BookStackSearchWidget.tsx b/frontend/src/components/dashboard/BookStackSearchWidget.tsx index 76207b4..8627496 100644 --- a/frontend/src/components/dashboard/BookStackSearchWidget.tsx +++ b/frontend/src/components/dashboard/BookStackSearchWidget.tsx @@ -14,6 +14,7 @@ import { Search, MenuBook } from '@mui/icons-material'; import { useQuery } from '@tanstack/react-query'; import { bookstackApi } from '../../services/bookstack'; import type { BookStackSearchResult } from '../../types/bookstack.types'; +import { safeOpenUrl } from '../../utils/safeOpenUrl'; function stripHtml(html: string): string { return html.replace(/<[^>]*>/g, '').trim(); @@ -28,7 +29,7 @@ const ResultRow: React.FC<{ result: BookStackSearchResult; showDivider: boolean return ( <> window.open(result.url, '_blank', 'noopener,noreferrer')} + onClick={() => safeOpenUrl(result.url)} sx={{ py: 1.5, px: 1, diff --git a/frontend/src/components/dashboard/NextcloudTalkWidget.tsx b/frontend/src/components/dashboard/NextcloudTalkWidget.tsx index 37300d9..de7b486 100644 --- a/frontend/src/components/dashboard/NextcloudTalkWidget.tsx +++ b/frontend/src/components/dashboard/NextcloudTalkWidget.tsx @@ -18,6 +18,7 @@ import { formatDistanceToNow } from 'date-fns'; import { de } from 'date-fns/locale'; import { nextcloudApi } from '../../services/nextcloud'; import type { NextcloudConversation } from '../../types/nextcloud.types'; +import { safeOpenUrl } from '../../utils/safeOpenUrl'; const POLL_INTERVAL = 2000; const POLL_TIMEOUT = 5 * 60 * 1000; @@ -27,7 +28,7 @@ const ConversationRow: React.FC<{ conversation: NextcloudConversation; showDivid showDivider, }) => { const handleClick = () => { - window.open(conversation.url, '_blank', 'noopener,noreferrer'); + safeOpenUrl(conversation.url); }; const relativeTime = conversation.lastMessage diff --git a/frontend/src/components/dashboard/VikunjaMyTasksWidget.tsx b/frontend/src/components/dashboard/VikunjaMyTasksWidget.tsx index 632aa6a..ac0ba08 100644 --- a/frontend/src/components/dashboard/VikunjaMyTasksWidget.tsx +++ b/frontend/src/components/dashboard/VikunjaMyTasksWidget.tsx @@ -14,6 +14,7 @@ import { format, isPast } from 'date-fns'; import { de } from 'date-fns/locale'; import { vikunjaApi } from '../../services/vikunja'; import type { VikunjaTask } from '../../types/vikunja.types'; +import { safeOpenUrl } from '../../utils/safeOpenUrl'; const PRIORITY_LABELS: Record = { 0: { label: 'Keine', color: 'default' }, @@ -30,7 +31,7 @@ const TaskRow: React.FC<{ task: VikunjaTask; showDivider: boolean; vikunjaUrl: s vikunjaUrl, }) => { const handleClick = () => { - window.open(`${vikunjaUrl}/tasks/${task.id}`, '_blank', 'noopener,noreferrer'); + safeOpenUrl(`${vikunjaUrl}/tasks/${task.id}`); }; const dueDateStr = task.due_date diff --git a/frontend/src/components/shared/NotificationBell.tsx b/frontend/src/components/shared/NotificationBell.tsx index 910a5ce..185362f 100644 --- a/frontend/src/components/shared/NotificationBell.tsx +++ b/frontend/src/components/shared/NotificationBell.tsx @@ -25,6 +25,20 @@ import type { Notification, NotificationSchwere } from '../../types/notification const POLL_INTERVAL_MS = 60_000; // 60 seconds +/** + * Only allow window.open for URLs whose origin matches the current app origin. + * External-looking URLs (different host or protocol-relative) are rejected to + * prevent open-redirect / tab-napping via notification link data from the backend. + */ +function isTrustedUrl(url: string): boolean { + try { + const parsed = new URL(url, window.location.origin); + return parsed.origin === window.location.origin; + } catch { + return false; + } +} + function schwerebColor(schwere: NotificationSchwere): 'error' | 'warning' | 'info' { if (schwere === 'fehler') return 'error'; if (schwere === 'warnung') return 'warning'; @@ -103,7 +117,11 @@ const NotificationBell: React.FC = () => { handleClose(); if (n.link) { if (n.link.startsWith('http://') || n.link.startsWith('https://')) { - window.open(n.link, '_blank'); + if (isTrustedUrl(n.link)) { + window.open(n.link, '_blank'); + } else { + console.warn('NotificationBell: blocked navigation to untrusted URL', n.link); + } } else { navigate(n.link); } diff --git a/frontend/src/pages/Login.tsx b/frontend/src/pages/Login.tsx index c9f798a..29b27a1 100644 --- a/frontend/src/pages/Login.tsx +++ b/frontend/src/pages/Login.tsx @@ -23,7 +23,11 @@ function Login() { useEffect(() => { if (isAuthenticated) { setIsRedirecting(true); - const from = (location.state as any)?.from || '/dashboard'; + const rawFrom = (location.state as any)?.from; + const from = + rawFrom && rawFrom.startsWith('/') && !rawFrom.startsWith('//') + ? rawFrom + : '/dashboard'; navigate(from, { replace: true }); } }, [isAuthenticated, navigate, location.state]); @@ -31,10 +35,11 @@ function Login() { const handleLogin = () => { try { // Persist the intended destination so LoginCallback can restore it - // after the full-page Authentik redirect round-trip - const from = (location.state as any)?.from; - if (from) { - sessionStorage.setItem('auth_redirect_from', from); + // after the full-page Authentik redirect round-trip. + // Validate that from is a safe internal path before storing it. + const rawFrom = (location.state as any)?.from; + if (rawFrom && rawFrom.startsWith('/') && !rawFrom.startsWith('//')) { + sessionStorage.setItem('auth_redirect_from', rawFrom); } const authUrl = authService.getAuthUrl(); window.location.href = authUrl; diff --git a/frontend/src/utils/config.ts b/frontend/src/utils/config.ts index 7e636ea..c2cb238 100644 --- a/frontend/src/utils/config.ts +++ b/frontend/src/utils/config.ts @@ -1,7 +1,18 @@ +const apiUrl: string = import.meta.env.VITE_API_URL; +const authentikUrl: string = import.meta.env.AUTHENTIK_URL || 'https://auth.firesuite.feuerwehr-rems.at'; +const clientId: string = import.meta.env.AUTHENTIK_CLIENT_ID; + +if (!apiUrl) { + console.error('Missing required environment variable: VITE_API_URL'); +} +if (!clientId) { + console.error('Missing required environment variable: AUTHENTIK_CLIENT_ID'); +} + export const config = { - apiUrl: import.meta.env.VITE_API_URL || 'http://localhost:3000', - authentikUrl: import.meta.env.AUTHENTIK_URL || 'https://auth.firesuite.feuerwehr-rems.at', - clientId: import.meta.env.AUTHENTIK_CLIENT_ID || 'your_client_id_here', + apiUrl, + authentikUrl, + clientId, }; export const API_URL = config.apiUrl; diff --git a/frontend/src/utils/safeOpenUrl.ts b/frontend/src/utils/safeOpenUrl.ts new file mode 100644 index 0000000..31e4b70 --- /dev/null +++ b/frontend/src/utils/safeOpenUrl.ts @@ -0,0 +1,20 @@ +/** + * Safely opens a URL in a new tab. + * + * Validates the URL before opening it to prevent malicious URLs (e.g. + * javascript: or data: URIs) from being opened if an API response is + * ever compromised. Only http: and https: URLs are allowed. + */ +export function safeOpenUrl(url: string): void { + try { + const parsed = new URL(url); + if (parsed.protocol !== 'https:' && parsed.protocol !== 'http:') { + console.warn(`safeOpenUrl: blocked URL with unexpected protocol "${parsed.protocol}": ${url}`); + return; + } + } catch { + console.warn(`safeOpenUrl: blocked invalid URL: ${url}`); + return; + } + window.open(url, '_blank', 'noopener,noreferrer'); +} diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index d96f86a..9c6cb3f 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -23,6 +23,6 @@ export default defineConfig({ envPrefix: ['VITE_', 'AUTHENTIK_'], build: { outDir: 'dist', - sourcemap: true, + sourcemap: false, }, });