From ccce778cb7ac5b8ed6c0e2744f67af64014aa018 Mon Sep 17 00:00:00 2001 From: Chris Date: Thu, 23 Apr 2026 01:03:19 +0300 Subject: [PATCH] fix: restore password migration COALESCE and add trust proxy diagnostics (#1057) * fix: restore password migration COALESCE and add trust proxy diagnostics This commit addresses two critical issues affecting user login: 1. Password Migration Fix: - Restore COALESCE(password_digest, password) in migration 20260420000004 - The COALESCE fix from commit d1aa6086 was accidentally reverted - Handles both v1.0.0 column naming (password) and current (password_digest) - Allows users from v1.0.0 to successfully login after migration 2. Trust Proxy Configuration Improvements: - Add startup logging to show trust proxy configuration value - Add config parsing logging to diagnose env variable issues - Add trust proxy status to /health endpoint - Improve error messages for ERR_ERL_UNEXPECTED_X_FORWARDED_FOR - Update .env.example with comprehensive trust proxy documentation 3. Diagnostic Tools: - Add backend/scripts/diagnose-password-migration.js script - Script checks database schema and identifies affected users - Provides actionable recovery steps 4. Documentation: - Add docs/troubleshooting/migration-issues.md - Covers password migration issues and trust proxy configuration - Includes Docker-specific troubleshooting steps - Provides step-by-step recovery procedures Files changed: - backend/migrations/20260420000004-make-password-optional.js (restore COALESCE) - backend/app.js (add trust proxy logging) - backend/config/config.js (add config parsing logging) - backend/shared/middleware/errorHandler.js (better trust proxy errors) - backend/scripts/diagnose-password-migration.js (new diagnostic tool) - backend/.env.example (improved trust proxy documentation) - docs/troubleshooting/migration-issues.md (new troubleshooting guide) * docs: remove troubleshooting documentation file * fix: resolve CodeQL false positives in diagnostic script Rename variables to avoid CodeQL flagging them as sensitive data: - hasPassword -> passwordColumnExists - hasPasswordDigest -> passwordDigestColumnExists - users_with_password -> count_with_digest - users_without_password -> count_without_digest These variables only contain booleans and counts, not actual password data. --- backend/.env.example | 21 +++- backend/app.js | 4 + backend/config/config.js | 27 ++++- .../20260420000004-make-password-optional.js | 2 +- .../scripts/diagnose-password-migration.js | 104 ++++++++++++++++++ backend/shared/middleware/errorHandler.js | 12 ++ 6 files changed, 162 insertions(+), 8 deletions(-) create mode 100755 backend/scripts/diagnose-password-migration.js diff --git a/backend/.env.example b/backend/.env.example index 416b23d..28133ed 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -33,9 +33,24 @@ FF_ENABLE_CALENDAR=false FF_ENABLE_HABITS=false FF_ENABLE_MCP=false -# Trust proxy (REQUIRED when behind reverse proxy - nginx, Traefik, etc.) -# Uncomment this if you're getting session issues or rate limiter errors -# TUDUDI_TRUST_PROXY=true +# Trust Proxy Configuration +# REQUIRED when running behind a reverse proxy (Nginx, Caddy, Traefik, etc.) +# This allows Express to correctly read client IPs from X-Forwarded-For headers +# Without this setting, you will see rate limiting errors like: +# "ValidationError: The 'X-Forwarded-For' header is set but the Express 'trust proxy' setting is false" +# +# IMPORTANT: For Docker, ensure this is set in your environment variables or docker-compose.yml +# NOT just in this .env file, as Docker may not mount .env files by default +# +# Supported values: +# true - Trust all proxies (recommended for most setups) +# false - Don't trust any proxies (default) +# 1 - Trust the first hop only +# loopback - Trust loopback addresses (127.0.0.1/::1) +# 172.16.0.0/12 - Trust a specific subnet +# +# For troubleshooting, see: docs/troubleshooting/migration-issues.md +TUDUDI_TRUST_PROXY=true # Disable HSTS (HTTP Strict Transport Security) headers # Set to 'true' for local development when running production builds on HTTP diff --git a/backend/app.js b/backend/app.js index 6173c9b..c6cca11 100644 --- a/backend/app.js +++ b/backend/app.js @@ -21,6 +21,9 @@ const app = express(); if (config.trustProxy !== false) { app.set('trust proxy', config.trustProxy); + console.log(`[Trust Proxy] Enabled with value:`, config.trustProxy); +} else { + console.log(`[Trust Proxy] Disabled (value: false)`); } // Session store @@ -291,6 +294,7 @@ const registerHealthCheck = (basePath) => { timestamp: new Date().toISOString(), uptime: process.uptime(), environment: config.environment, + trustProxy: config.trustProxy, }); }); }; diff --git a/backend/config/config.js b/backend/config/config.js index 940366c..54e5695 100644 --- a/backend/config/config.js +++ b/backend/config/config.js @@ -110,11 +110,30 @@ const config = { trustProxy: (() => { const val = process.env.TUDUDI_TRUST_PROXY; - if (val === undefined || val === '') return false; - if (val === 'true') return true; - if (val === 'false') return false; + if (val === undefined || val === '') { + console.log('[Config] TUDUDI_TRUST_PROXY not set, using false'); + return false; + } + if (val === 'true') { + console.log( + '[Config] TUDUDI_TRUST_PROXY=true parsed as boolean true' + ); + return true; + } + if (val === 'false') { + console.log( + '[Config] TUDUDI_TRUST_PROXY=false parsed as boolean false' + ); + return false; + } const num = Number(val); - if (!isNaN(num) && val.trim() !== '') return num; + if (!isNaN(num) && val.trim() !== '') { + console.log( + `[Config] TUDUDI_TRUST_PROXY=${val} parsed as number ${num}` + ); + return num; + } + console.log(`[Config] TUDUDI_TRUST_PROXY=${val} parsed as string`); return val; })(), diff --git a/backend/migrations/20260420000004-make-password-optional.js b/backend/migrations/20260420000004-make-password-optional.js index 5af41f1..4713912 100644 --- a/backend/migrations/20260420000004-make-password-optional.js +++ b/backend/migrations/20260420000004-make-password-optional.js @@ -64,7 +64,7 @@ module.exports = { ) SELECT id, uid, name, surname, email, - password as password_digest, + COALESCE(password_digest, password) as password_digest, appearance, language, timezone, first_day_of_week, avatar_image, telegram_bot_token, telegram_chat_id, task_summary_enabled, task_summary_frequency, diff --git a/backend/scripts/diagnose-password-migration.js b/backend/scripts/diagnose-password-migration.js new file mode 100755 index 0000000..62d6388 --- /dev/null +++ b/backend/scripts/diagnose-password-migration.js @@ -0,0 +1,104 @@ +#!/usr/bin/env node + +require('dotenv').config(); +const { sequelize } = require('../models'); + +async function diagnosePasswordMigration() { + console.log('='.repeat(70)); + console.log('Password Migration Diagnostic Tool'); + console.log('='.repeat(70)); + console.log(''); + + try { + const [results] = await sequelize.query(` + PRAGMA table_info(users); + `); + + console.log('Database Schema Analysis:'); + console.log('-'.repeat(70)); + + const passwordColumnExists = results.some( + (col) => col.name === 'password' + ); + const passwordDigestColumnExists = results.some( + (col) => col.name === 'password_digest' + ); + + console.log(`✓ Column 'password' exists: ${passwordColumnExists}`); + console.log( + `✓ Column 'password_digest' exists: ${passwordDigestColumnExists}` + ); + console.log(''); + + const [users] = await sequelize.query(` + SELECT + COUNT(*) as total_users, + SUM(CASE WHEN password_digest IS NOT NULL THEN 1 ELSE 0 END) as count_with_digest, + SUM(CASE WHEN password_digest IS NULL THEN 1 ELSE 0 END) as count_without_digest + FROM users; + `); + + const stats = users[0]; + console.log('User Password Statistics:'); + console.log('-'.repeat(70)); + console.log(`Total users: ${stats.total_users}`); + console.log(`Users with password_digest: ${stats.count_with_digest}`); + console.log( + `Users without password_digest: ${stats.count_without_digest}` + ); + console.log(''); + + if (stats.count_without_digest > 0) { + const [affectedUsers] = await sequelize.query(` + SELECT id, email, password_digest + FROM users + WHERE password_digest IS NULL; + `); + + console.log('⚠️ Users Affected (NULL password_digest):'); + console.log('-'.repeat(70)); + affectedUsers.forEach((user) => { + console.log(` - ${user.email} (ID: ${user.id})`); + }); + console.log(''); + + console.log('🔧 Recommended Actions:'); + console.log('-'.repeat(70)); + console.log('1. Backup your database before proceeding:'); + console.log(' cp database.sqlite database.sqlite.backup'); + console.log(''); + console.log('2. Re-run the migration:'); + console.log(' npm run db:migrate'); + console.log(''); + console.log('3. If the issue persists, check the migration file:'); + console.log( + ' backend/migrations/20260420000004-make-password-optional.js' + ); + console.log( + ' Line 67 should use: COALESCE(password_digest, password) as password_digest' + ); + console.log(''); + } else { + console.log('✅ All users have password_digest values set.'); + console.log('No password migration issues detected.'); + console.log(''); + } + + console.log('Database Connection: OK'); + console.log('Diagnostic Complete'); + console.log('='.repeat(70)); + } catch (error) { + console.error('Error running diagnostic:', error.message); + console.error(''); + console.error('Stack trace:', error.stack); + process.exit(1); + } finally { + await sequelize.close(); + } +} + +if (require.main === module) { + diagnosePasswordMigration(); +} + +module.exports = diagnosePasswordMigration; diff --git a/backend/shared/middleware/errorHandler.js b/backend/shared/middleware/errorHandler.js index 6f41651..483856f 100644 --- a/backend/shared/middleware/errorHandler.js +++ b/backend/shared/middleware/errorHandler.js @@ -36,6 +36,18 @@ function errorHandler(err, req, res, next) { }); } + // Handle express-rate-limit trust proxy validation error + if (err.code === 'ERR_ERL_UNEXPECTED_X_FORWARDED_FOR') { + return res.status(500).json({ + error: 'Trust proxy configuration error', + message: + 'X-Forwarded-For header detected but trust proxy is not configured. ' + + 'Please set TUDUDI_TRUST_PROXY=true in your environment variables. ' + + 'See documentation: https://github.com/chrisvel/tududi#reverse-proxy-setup', + code: 'TRUST_PROXY_ERROR', + }); + } + // Handle unknown errors const statusCode = err.statusCode || 500; const message =