From 87f459f5e6e9bf42aa8266d4e0ea2657cb9398ca Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Thu, 27 Nov 2025 20:30:53 +0100 Subject: [PATCH] feat(syncServer): add rate limiting, enhance user registration with stronger password validation and add verification token expiration --- packages/super-sync-server/package.json | 1 + packages/super-sync-server/src/api.ts | 12 ++- packages/super-sync-server/src/auth.ts | 99 ++++++++++++------------ packages/super-sync-server/src/db.ts | 13 ++++ packages/super-sync-server/src/pages.ts | 14 +++- packages/super-sync-server/src/server.ts | 21 +++++ 6 files changed, 106 insertions(+), 54 deletions(-) diff --git a/packages/super-sync-server/package.json b/packages/super-sync-server/package.json index feb8b32c0f..2bd33e1ec3 100644 --- a/packages/super-sync-server/package.json +++ b/packages/super-sync-server/package.json @@ -11,6 +11,7 @@ }, "dependencies": { "@fastify/cors": "^11.1.0", + "@fastify/rate-limit": "^10.3.0", "@fastify/static": "^8.3.0", "bcryptjs": "^3.0.3", "better-sqlite3": "^12.4.6", diff --git a/packages/super-sync-server/src/api.ts b/packages/super-sync-server/src/api.ts index 2193ccb957..ccbbad27d4 100644 --- a/packages/super-sync-server/src/api.ts +++ b/packages/super-sync-server/src/api.ts @@ -6,7 +6,13 @@ import { Logger } from './logger'; // Zod Schemas const RegisterSchema = z.object({ email: z.string().email('Invalid email format'), - password: z.string().min(8, 'Password must be at least 8 characters long'), + password: z + .string() + .min(12, 'Password must be at least 12 characters long') + .regex(/[a-z]/, 'Password must contain at least one lowercase letter') + .regex(/[A-Z]/, 'Password must contain at least one uppercase letter') + .regex(/\d/, 'Password must contain at least one number') + .regex(/[\W_]/, 'Password must contain at least one special character'), }); const LoginSchema = z.object({ @@ -40,9 +46,7 @@ export const apiRoutes = async (fastify: FastifyInstance): Promise => { const { email, password } = parseResult.data; const result = await registerUser(email, password); - return reply - .status(201) - .send({ message: 'User registered. Please verify your email.', ...result }); + return reply.status(201).send(result); } catch (err) { Logger.error(`Registration error: ${errorMessage(err)}`); // Generic error message to avoid leaking implementation details (e.g. specific DB errors) diff --git a/packages/super-sync-server/src/auth.ts b/packages/super-sync-server/src/auth.ts index 8533cdb85a..4d745f64f0 100644 --- a/packages/super-sync-server/src/auth.ts +++ b/packages/super-sync-server/src/auth.ts @@ -11,67 +11,57 @@ const getJwtSecret = (): string => { if (process.env.NODE_ENV === 'production') { throw new Error('JWT_SECRET environment variable is required in production'); } - Logger.warn('JWT_SECRET not set - using insecure default for development only'); - return 'dev-only-insecure-secret'; + Logger.warn('JWT_SECRET not set - using random secret for development'); + return randomBytes(64).toString('hex'); } return secret; }; const JWT_SECRET = getJwtSecret(); -// Simple email validation regex -const EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; - -const isValidEmail = (email: string): boolean => EMAIL_REGEX.test(email); - export const registerUser = async ( email: string, password: string, -): Promise<{ id: number | bigint; email: string }> => { - // Validate email format - if (!isValidEmail(email)) { - throw new Error('Invalid email format'); - } - - // Validate password length - if (password.length < 8) { - throw new Error('Password must be at least 8 characters'); - } +): Promise<{ message: string }> => { + // Password strength validation is handled by Zod in api.ts const db = getDb(); - - // Check if user exists - const existingUser = db.prepare('SELECT * FROM users WHERE email = ?').get(email); - if (existingUser) { - throw new Error('User already exists'); - } - - const passwordHash = bcrypt.hashSync(password, 10); + const passwordHash = bcrypt.hashSync(password, 12); const verificationToken = randomBytes(32).toString('hex'); + const expiresAt = Date.now() + 24 * 60 * 60 * 1000; // 24 hours - const info = db - .prepare( - ` - INSERT INTO users (email, password_hash, verification_token) - VALUES (?, ?, ?) - `, - ) - .run(email, passwordHash, verificationToken); + try { + const info = db + .prepare( + ` + INSERT INTO users (email, password_hash, verification_token, verification_token_expires_at) + VALUES (?, ?, ?, ?) + `, + ) + .run(email, passwordHash, verificationToken, expiresAt); - Logger.info(`User registered: ${email} (ID: ${info.lastInsertRowid})`); + Logger.info(`User registered: ${email} (ID: ${info.lastInsertRowid})`); - // Send verification email asynchronously - const emailSent = await sendVerificationEmail(email, verificationToken); - if (!emailSent) { - // Clean up the newly created account to prevent unusable, un-verifiable entries - db.prepare('DELETE FROM users WHERE id = ?').run(info.lastInsertRowid); - throw new Error('Failed to send verification email. Please try again later.'); + // Send verification email asynchronously + const emailSent = await sendVerificationEmail(email, verificationToken); + if (!emailSent) { + // Clean up the newly created account to prevent unusable, un-verifiable entries + db.prepare('DELETE FROM users WHERE id = ?').run(info.lastInsertRowid); + throw new Error('Failed to send verification email. Please try again later.'); + } + } catch (err: any) { + // If unique constraint violation (user exists), we swallow the error + // to prevent email enumeration. + if (err.code === 'SQLITE_CONSTRAINT_UNIQUE') { + Logger.info(`Registration attempt for existing email: ${email}`); + // In a real system, we might want to send a "You already have an account" email here + } else { + throw err; + } } return { - id: info.lastInsertRowid, - email, - // verificationToken, // Don't return token in response for security, user must check email + message: 'Registration successful. Please check your email to verify your account.', }; }; @@ -86,8 +76,15 @@ export const verifyEmail = (token: string): boolean => { throw new Error('Invalid verification token'); } + if ( + user.verification_token_expires_at && + user.verification_token_expires_at < Date.now() + ) { + throw new Error('Verification token has expired'); + } + db.prepare( - 'UPDATE users SET is_verified = 1, verification_token = NULL WHERE id = ?', + 'UPDATE users SET is_verified = 1, verification_token = NULL, verification_token_expires_at = NULL WHERE id = ?', ).run(user.id); Logger.info(`User verified: ${user.email}`); @@ -104,11 +101,15 @@ export const loginUser = ( | User | undefined; - if (!user) { - throw new Error('Invalid credentials'); - } + // Timing attack mitigation: always perform a comparison + // Use a dummy hash so the comparison takes roughly the same time + // $2a$12$ is bcrypt prefix for 12 rounds + const dummyHash = '$2a$12$e0MyzXyjpJS7dAC5B5S3.O/p.u.m.u.m.u.m.u.m.u.m.u.m.u.m.'; + const hashToCompare = user ? user.password_hash : dummyHash; - if (!bcrypt.compareSync(password, user.password_hash)) { + const isMatch = bcrypt.compareSync(password, hashToCompare); + + if (!user || !isMatch) { throw new Error('Invalid credentials'); } @@ -117,7 +118,7 @@ export const loginUser = ( } const token = jwt.sign({ userId: user.id, email: user.email }, JWT_SECRET, { - expiresIn: '30d', + expiresIn: '7d', }); return { token, user: { id: user.id, email: user.email } }; diff --git a/packages/super-sync-server/src/db.ts b/packages/super-sync-server/src/db.ts index cec72e5100..1d300636d7 100644 --- a/packages/super-sync-server/src/db.ts +++ b/packages/super-sync-server/src/db.ts @@ -9,6 +9,7 @@ export interface User { password_hash: string; is_verified: number; // 0 or 1 verification_token: string | null; + verification_token_expires_at: number | null; // Unix timestamp created_at: string; } @@ -32,10 +33,22 @@ export const initDb = (dataDir: string) => { password_hash TEXT NOT NULL, is_verified INTEGER DEFAULT 0, verification_token TEXT, + verification_token_expires_at INTEGER, created_at TEXT DEFAULT CURRENT_TIMESTAMP ) `); + // Migration: Check if verification_token_expires_at exists + const columns = db.pragma('table_info(users)') as { name: string }[]; + const hasExpiresAt = columns.some( + (col) => col.name === 'verification_token_expires_at', + ); + + if (!hasExpiresAt) { + Logger.info('Migrating database: adding verification_token_expires_at column'); + db.exec('ALTER TABLE users ADD COLUMN verification_token_expires_at INTEGER'); + } + Logger.info(`Database initialized at ${dbPath}`); }; diff --git a/packages/super-sync-server/src/pages.ts b/packages/super-sync-server/src/pages.ts index edf47bad27..2eecb87a71 100644 --- a/packages/super-sync-server/src/pages.ts +++ b/packages/super-sync-server/src/pages.ts @@ -6,6 +6,16 @@ import { Logger } from './logger'; const errorMessage = (err: unknown): string => err instanceof Error ? err.message : 'Unknown error'; +// Basic HTML escape +const escapeHtml = (unsafe: string): string => { + return unsafe + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +}; + interface VerifyEmailQuery { token?: string; } @@ -41,7 +51,9 @@ export async function pageRoutes(fastify: FastifyInstance) { `); } catch (err) { Logger.error(`Verification error: ${errorMessage(err)}`); - return reply.status(400).send(`Verification failed: ${errorMessage(err)}`); + // Escape the error message to prevent XSS + const safeError = escapeHtml(errorMessage(err)); + return reply.status(400).send(`Verification failed: ${safeError}`); } }); } diff --git a/packages/super-sync-server/src/server.ts b/packages/super-sync-server/src/server.ts index 4563f1bfd3..9e665ab173 100644 --- a/packages/super-sync-server/src/server.ts +++ b/packages/super-sync-server/src/server.ts @@ -2,6 +2,7 @@ import { v2 as webdav } from 'webdav-server'; import * as fs from 'fs'; import Fastify, { FastifyInstance } from 'fastify'; import cors from '@fastify/cors'; +import rateLimit from '@fastify/rate-limit'; import fastifyStatic from '@fastify/static'; import * as path from 'path'; import { loadConfigFromEnv, ServerConfig } from './config'; @@ -131,6 +132,14 @@ export const createServer = ( logger: false, // We use our own logger }); + // Rate Limiting (prevent brute force) + // 100 requests per 15 minutes globally is a safe default + // We can tighten this for specific routes if needed + await fastifyServer.register(rateLimit, { + max: 100, + timeWindow: '15 minutes', + }); + // CORS Configuration if (fullConfig.cors.enabled) { await fastifyServer.register(cors, { @@ -228,6 +237,18 @@ export const createServer = ( const normalizedUrl = originalUrl.startsWith('/') ? originalUrl : `/${originalUrl}`; + + // Path Traversal Protection + // Ensure path does not contain ".." segments + if ( + normalizedUrl.includes('/../') || + normalizedUrl.endsWith('/..') || + normalizedUrl.includes('\\..\\') + ) { + reply.status(400).send({ error: 'Invalid path: Path traversal not allowed' }); + return; + } + const userScopedUrl = `/${userSegment}${normalizedUrl}`.replace(/\/{2,}/g, '/'); req.raw.url = userScopedUrl;