From 542b35d6d4ad62e1385c7a7b5162a89633759b44 Mon Sep 17 00:00:00 2001 From: abiteman <30483819+abiteman@users.noreply.github.com> Date: Thu, 19 Jun 2025 21:37:59 -0500 Subject: [PATCH] feat: fix PIN input errors and improve security configuration - Add ALLOWED_ORIGINS environment variable for flexible CORS configuration * Support both localhost and remote access for production deployments * Default includes common localhost ports (3000, 5050) and 127.0.0.1 variants - Replace Toastify CDN with local node module dependency * Install toastify-js as local dependency * Serve toastify files from /toastify static route * Remove external CDN dependency for better security and reliability - Update Content Security Policy (CSP) configuration * Remove cdn.jsdelivr.net from allowed sources * Add additional security headers (Referrer-Policy, base-uri, object-src) * Maintain necessary unsafe-inline permissions for current inline styles - Implement server-side controlled login redirects * Update /api/auth/verify-pin to return redirectUrl in response * Replace hardcoded client-side redirects with server-provided URLs * Prevent potential race conditions in authentication flow - Fix toastify static file serving path (src/ not dist/) Fixes: PIN input errors preventing login (GitHub issue #62) Improves: Security posture, eliminates external dependencies, CORS flexibility Breaking changes: Requires ALLOWED_ORIGINS environment variable for production CORS configuration --- package-lock.json | 9 ++++++++- package.json | 3 ++- public/index.html | 4 ++-- public/login.html | 4 +++- src/app.js | 35 ++++++++++++++++++++++------------- src/config/index.js | 23 +++++++++++++++++++++++ src/middleware/security.js | 13 +++++++++---- src/routes/auth.js | 16 ++++++++++++++-- 8 files changed, 83 insertions(+), 24 deletions(-) diff --git a/package-lock.json b/package-lock.json index 87e9b0d..f1b9558 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,7 +17,8 @@ "dotenv": "^16.0.3", "express": "^4.18.2", "express-rate-limit": "^7.1.5", - "multer": "^1.4.5-lts.1" + "multer": "^1.4.5-lts.1", + "toastify-js": "^1.12.0" }, "devDependencies": { "eslint": "^8.56.0", @@ -4301,6 +4302,12 @@ "node": ">=8.0" } }, + "node_modules/toastify-js": { + "version": "1.12.0", + "resolved": "https://registry.npmjs.org/toastify-js/-/toastify-js-1.12.0.tgz", + "integrity": "sha512-HeMHCO9yLPvP9k0apGSdPUWrUbLnxUKNFzgUoZp1PHCLploIX/4DSQ7V8H25ef+h4iO9n0he7ImfcndnN6nDrQ==", + "license": "MIT" + }, "node_modules/toidentifier": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/toidentifier/-/toidentifier-1.0.1.tgz", diff --git a/package.json b/package.json index b41aff1..58e6fb5 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,8 @@ "dotenv": "^16.0.3", "express": "^4.18.2", "express-rate-limit": "^7.1.5", - "multer": "^1.4.5-lts.1" + "multer": "^1.4.5-lts.1", + "toastify-js": "^1.12.0" }, "devDependencies": { "eslint": "^8.56.0", diff --git a/public/index.html b/public/index.html index 1bec4bc..ea703e9 100644 --- a/public/index.html +++ b/public/index.html @@ -5,8 +5,8 @@ {{SITE_TITLE}} - Simple File Upload - - + + diff --git a/public/login.html b/public/login.html index c5fc37a..c941f7f 100644 --- a/public/login.html +++ b/public/login.html @@ -135,7 +135,9 @@ // Simplified success and error handling if (data.success) { - window.location.href = '/'; + // Use server-provided redirect URL to prevent race conditions + const redirectUrl = data.redirectUrl || '/'; + window.location.href = redirectUrl; } else { // Show error message document.getElementById('pin-error').textContent = data.error || 'Authentication failed'; diff --git a/src/app.js b/src/app.js index 97349d9..ebb2fd0 100644 --- a/src/app.js +++ b/src/app.js @@ -41,8 +41,23 @@ const app = express(); // Trust proxy headers (important for rate limiting and secure cookies if behind proxy) app.set('trust proxy', 1); // Adjust the number based on your proxy setup depth -// --- Middleware Setup --- -app.use(cors()); // TODO: Configure CORS more strictly for production if needed +// Middleware setup +app.use(cors({ + origin: function (origin, callback) { + // Allow requests with no origin (like mobile apps, Postman) + if (!origin) return callback(null, true); + + if (config.allowedOrigins.indexOf(origin) !== -1) { + return callback(null, true); + } else { + logger.warn(`CORS blocked request from origin: ${origin}`); + return callback(new Error('Not allowed by CORS')); + } + }, + credentials: true, + methods: ['GET', 'POST', 'PUT', 'DELETE', 'OPTIONS'], + allowedHeaders: ['Content-Type', 'Authorization', 'X-Pin', 'X-Batch-ID'] +})); app.use(cookieParser()); app.use(express.json()); // For parsing application/json app.use(securityHeaders); // Apply security headers @@ -136,19 +151,13 @@ app.get('/login.html', (req, res) => { } }); -// --- Health Check Endpoint --- -app.get('/health', (req, res) => { - res.status(200).json({ status: 'UP', message: 'Server is healthy' }); -}); +// Serve remaining static files +app.use(express.static('public')); -// --- Static File Serving --- -// Serve static files (CSS, JS, assets) from the 'public' directory -// Use express.static middleware, placed AFTER specific HTML routes -app.use(express.static(path.join(__dirname, '../public'))); +// Serve toastify files from node_modules +app.use('/toastify', express.static(path.join(__dirname, '../node_modules/toastify-js/src'))); - -// --- Error Handling Middleware --- -// Catch-all for unhandled errors +// Error handling middleware app.use((err, req, res, next) => { // eslint-disable-line no-unused-vars logger.error(`Unhandled application error: ${err.message}`, err.stack); // Avoid sending stack trace in production diff --git a/src/config/index.js b/src/config/index.js index c711d10..35feb1b 100644 --- a/src/config/index.js +++ b/src/config/index.js @@ -122,6 +122,29 @@ const config = { allowedExtensions: process.env.ALLOWED_EXTENSIONS ? process.env.ALLOWED_EXTENSIONS.split(',').map(ext => ext.trim().toLowerCase().replace(/^\./, '.')).filter(Boolean) : null, + + /** + * Allowed CORS origins (comma-separated, optional) + * Set via ALLOWED_ORIGINS in .env + * Defaults to localhost and 127.0.0.1 variants if not specified + */ + allowedOrigins: process.env.ALLOWED_ORIGINS ? + process.env.ALLOWED_ORIGINS.split(',').map(origin => origin.trim()).filter(Boolean) : + [ + 'http://localhost:3000', + 'http://127.0.0.1:3000', + 'http://localhost:5050', + 'http://127.0.0.1:5050' + ], + + allowedIframeOrigins: process.env.ALLOWED_IFRAME_ORIGINS + ? process.env.ALLOWED_IFRAME_ORIGINS.split(',').map(origin => origin.trim()).filter(Boolean) + : null, + + /** + * Max number of retries for client-side chunk uploads (default: 5) + * Set via CLIENT_MAX_RETRIES in .env + */ clientMaxRetries: (() => { const retries = parseInt(process.env.CLIENT_MAX_RETRIES || DEFAULT_CLIENT_MAX_RETRIES, 10); return (isNaN(retries) || retries < 0) ? DEFAULT_CLIENT_MAX_RETRIES : retries; diff --git a/src/middleware/security.js b/src/middleware/security.js index f015256..e8ce0f5 100644 --- a/src/middleware/security.js +++ b/src/middleware/security.js @@ -12,13 +12,17 @@ const { config } = require('../config'); * Security headers middleware */ function securityHeaders(req, res, next) { - // Content Security Policy + // Content Security Policy - Updated to fix PIN input issues + // We're temporarily keeping unsafe-inline for styles until we move all inline styles let csp = "default-src 'self'; " + "connect-src 'self'; " + - "style-src 'self' 'unsafe-inline' cdn.jsdelivr.net; " + - "script-src 'self' 'unsafe-inline' cdn.jsdelivr.net; " + - "img-src 'self' data: blob:;"; + "style-src 'self' 'unsafe-inline'; " + + "script-src 'self' 'unsafe-inline'; " + + "img-src 'self' data: blob:; " + + "font-src 'self' data:; " + + "object-src 'none'; " + + "base-uri 'self';"; // If allowedIframeOrigins is set, allow those origins to embed via iframe if (config.allowedIframeOrigins && config.allowedIframeOrigins.length > 0) { @@ -34,6 +38,7 @@ function securityHeaders(req, res, next) { res.setHeader('Content-Security-Policy', csp); res.setHeader('X-Content-Type-Options', 'nosniff'); res.setHeader('X-XSS-Protection', '1; mode=block'); + res.setHeader('Referrer-Policy', 'strict-origin-when-cross-origin'); // Strict Transport Security (when in production) if (process.env.NODE_ENV === 'production') { diff --git a/src/routes/auth.js b/src/routes/auth.js index 7662701..bb52806 100644 --- a/src/routes/auth.js +++ b/src/routes/auth.js @@ -28,7 +28,12 @@ router.post('/verify-pin', (req, res) => { sameSite: 'strict', path: '/' }); - return res.json({ success: true, error: null }); + const redirectUrl = req.query.redirect || '/'; + return res.json({ + success: true, + error: null, + redirectUrl: redirectUrl + }); } // Validate PIN format @@ -69,7 +74,14 @@ router.post('/verify-pin', (req, res) => { }); logger.info(`Successful PIN verification from IP: ${ip}`); - res.json({ success: true, error: null }); + + // Return success with redirect URL controlled by server + const redirectUrl = req.query.redirect || '/'; + res.json({ + success: true, + error: null, + redirectUrl: redirectUrl + }); } else { // Record failed attempt const attempts = recordAttempt(ip);