fix: remove duplicate login route (authLogin.js), consolidate into auth.js

- Deleted routes/authLogin.js (orphaned duplicate login handler)
- Removed authLoginRouter import and mount from server.js
- Rate limiter now runs as standalone middleware on /api/auth/login
- Added try/catch to auth.js login handler (was only in deleted file)
- Consistent audit log variable naming (username vs req.body.username)
- No functionality change — login flow works identically

FUTURE.md

@@ -34,11 +34,206 @@ Items are grouped under their priority section heading (`## 🔴 CRITICAL`, `##
 ### 🔴 CRITICAL
 
 
+### 🔴 Notification Runner Leaks Bill Details Across Users — CRITICAL
+**Priority:** CRITICAL  
+**Added:** 2026-05-10 by Prime (code review)  
+**Type:** SECURITY
 
+**Description:**
+`services/notificationService.js` loads every active bill globally (`SELECT * FROM bills WHERE active = 1`) and then loops every notification recipient. In per-user notification mode, this can email one user's bill names, due dates, and amounts to every other opted-in user because the recipient is never matched to `bill.user_id`.
+
+**Affected Files:**
+- `services/notificationService.js:180-233`
+
+**Potential Fix:**
+Fetch bills per recipient with `WHERE user_id = ?`, or group active bills by `user_id` and only send each bill to its owner. Add a regression test with two users to prove cross-user notifications cannot occur.
+
+**Severity:** CRITICAL
+
+
+### 🟠 HIGH
+
+### 🟠 Admin Can Toggle Payments on Any User Bill — HIGH
+**Priority:** HIGH  
+**Added:** 2026-05-10 by Prime (code review)  
+**Type:** SECURITY
+
+**Description:**
+`POST /api/bills/:id/toggle-paid` has a special admin branch that selects bills without `user_id = req.user.id`, then soft-deletes or creates payments on that bill. The surrounding router is mounted with `requireUser`, and `requireUser` allows `role='admin'`, so a non-default admin can modify another user's financial records through this endpoint. This contradicts the documented admin/privacy boundary used elsewhere.
+
+**Affected Files:**
+- `routes/bills.js:393-443`
+- `middleware/requireAuth.js:38-45`
+- `server.js:86-87`
+
+**Potential Fix:**
+Remove the admin-wide branch and require ownership for all bill payment mutations. If admin support access is intentional, put it behind a separate audited admin endpoint with explicit UI/permission labeling.
+
+**Severity:** HIGH
+
+### 🟠 Analytics Validation Errors Crash Instead of Returning 400 — HIGH
+**Priority:** HIGH  
+**Added:** 2026-05-10 by Prime (code review)  
+**Type:** BUG / API_CONTRACT
+
+**Description:**
+`routes/analytics.js` calls `standardizeError()` on invalid query parameters, but never imports it. Valid analytics requests work, but invalid `year`, `month`, `months`, `category_id`, or `bill_id` input will throw `ReferenceError: standardizeError is not defined` and return a 500 instead of a standardized 400 response.
+
+**Affected Files:**
+- `routes/analytics.js:1-3`
+- `routes/analytics.js:97-99`
+
+**Potential Fix:**
+Import `standardizeError` from `../middleware/errorFormatter` and add route tests for invalid analytics query parameters.
+
+**Severity:** HIGH
+
+### 🟠 User Export Drops Recurrence and History-Range Data — HIGH
+**Priority:** HIGH  
+**Added:** 2026-05-10 by Prime (code review)  
+**Type:** DATA_INTEGRITY
+
+**Description:**
+`routes/export.js` does not include `bills.cycle_type`, `bills.cycle_day`, or `bill_history_ranges` in user Excel/SQLite exports. Users who rely on exports as portable backups can lose non-monthly recurrence settings and history visibility ranges after exporting and re-importing their data.
+
+**Affected Files:**
+- `routes/export.js`
+- `db/schema.sql`
+
+**Potential Fix:**
+Add `cycle_type` and `cycle_day` to exported bill rows and the SQLite export schema. Export `bill_history_ranges` as its own Excel sheet/SQLite table and update the matching import path to restore those rows under the current user.
+
+**Severity:** HIGH
+
+### 🟠 Single-User Mode Can Lock Itself Out When Expired Sessions Exist — HIGH
+**Priority:** HIGH  
+**Added:** 2026-05-10 by Prime (code review)  
+**Type:** BUG
+
+**Description:**
+`getSingleModeUser()` uses a `LEFT JOIN sessions` and requires `(s.expires_at > datetime('now') OR s.id IS NULL)`. If the configured default single-mode user has only expired session rows, the join returns expired rows, `s.id` is not NULL, and no user is returned. That defeats the intended session bypass for single-user mode until expired sessions are pruned.
+
+**Affected Files:**
+- `middleware/requireAuth.js`
+- `services/authService.js`
+
+**Potential Fix:**
+Do not join sessions for single-user mode. Validate only that the configured default user exists, is active, and has role `user`, or explicitly prune expired sessions before checking.
+
+**Severity:** HIGH
 
 
 ### 🟡 MEDIUM
 
+
+### 🟡 Password Change Rate Limiter Applies to Every Profile Endpoint — MEDIUM
+**Priority:** MEDIUM  
+**Added:** 2026-05-10 by Prime (code review)  
+**Type:** API_CONTRACT / CONFIG
+
+**Description:**
+`server.js` mounts `passwordLimiter` across all `/api/profile` routes, not only `/api/profile/change-password`. Five harmless profile reads/settings updates in 15 minutes can block profile access with a password-change rate-limit message, while also making profile pages fragile under normal UI polling or retries.
+
+**Affected Files:**
+- `server.js:102-103`
+- `middleware/rateLimiter.js:18-23`
+- `routes/profile.js`
+
+**Potential Fix:**
+Apply `passwordLimiter` only to `POST /api/profile/change-password` (or inside the route), leaving profile reads/settings on a normal API limiter if needed.
+
+**Severity:** MEDIUM
+
+### 🟡 Profile Password Change Does Not Invalidate Other Sessions — MEDIUM
+**Priority:** MEDIUM  
+**Added:** 2026-05-10 by Prime (code review)  
+**Type:** SECURITY
+
+**Description:**
+`POST /api/profile/change-password` only invalidates other sessions and rotates the current session when `req.sessionId` exists, but `requireAuth()` never sets `req.sessionId`. Password changes through the profile endpoint can therefore leave all existing sessions active and skip current-session rotation. The separate `/api/auth/change-password` route uses the cookie value directly and does not have this specific gap.
+
+**Affected Files:**
+- `routes/profile.js:211-223`
+- `middleware/requireAuth.js:22-31`
+- `services/authService.js:175-194`
+
+**Potential Fix:**
+Set `req.sessionId` in `requireAuth()` from the session cookie after successful authentication, or make the profile route use `req.cookies?.[COOKIE_NAME]` consistently with `/api/auth/change-password`. Add a regression test that verifies other sessions are removed after profile password change.
+
+**Severity:** MEDIUM
+
+### 🟡 CSRF Defaults Conflict with SPA Token Loading — MEDIUM
+**Priority:** MEDIUM  
+**Added:** 2026-05-10 by Prime (code review)  
+**Type:** CONFIG / SECURITY
+
+**Description:**
+The CSRF middleware defaults the CSRF cookie to `httpOnly: true`, but the SPA reads `bt_csrf_token` from `document.cookie` and sends it in `x-csrf-token`. With default settings, state-changing requests from the client will not include the token. `docker-compose.yml` compensates with `CSRF_HTTP_ONLY=false`, which means the secure default and the actual SPA contract disagree.
+
+**Affected Files:**
+- `middleware/csrf.js:12-16`
+- `middleware/csrf.js:42-52`
+- `client/api.js:1-16`
+- `docker-compose.yml`
+
+**Potential Fix:**
+Choose one CSRF pattern and align defaults/docs: either intentionally use a readable double-submit token for the SPA, or provide a `/api/csrf-token` endpoint/header bootstrap so the cookie can stay HTTP-only.
+
+**Severity:** MEDIUM
+
+### 🟡 Change-Password Routes Are Globally Exempted from CSRF — MEDIUM
+**Priority:** MEDIUM  
+**Added:** 2026-05-10 by Prime (code review)  
+**Type:** SECURITY
+
+**Description:**
+`server.js` sets `req.csrfSkip = true` for `/api/auth/change-password` and `/api/profile/change-password`. These routes still require the current password, so this is not an immediate account-takeover issue, but password-changing is a sensitive state mutation and should not be excluded from the same CSRF protection as other authenticated writes.
+
+**Affected Files:**
+- `server.js:70-71`
+- `routes/auth.js:116-156`
+- `routes/profile.js:177-230`
+
+**Potential Fix:**
+Remove the global CSRF skip for password-change routes once the SPA token flow is aligned, and keep only login/OIDC bootstrap routes exempt where no authenticated session exists yet.
+
+**Severity:** MEDIUM
+
+### 🟡 Notification Due-Day Math Can Miss Same-Day Reminders — MEDIUM
+**Priority:** MEDIUM  
+**Added:** 2026-05-10 by Prime (code review)  
+**Type:** BUG
+
+**Description:**
+`runNotifications()` computes `diffDays` by subtracting the current timestamp from midnight on the due date and flooring the result. After midnight has passed on the due date, the difference becomes a small negative value and floors to `-1`, so due-today bills can be classified as overdue instead of `due_today`. Similar timezone/partial-day drift can affect 1-day and 3-day reminders.
+
+**Affected Files:**
+- `services/notificationService.js:172-175`
+- `services/notificationService.js:213-222`
+
+**Potential Fix:**
+Normalize both dates to local date-only midnight or compare ISO date strings/calendar days before calculating reminder type. Add tests around morning/afternoon due-today execution.
+
+**Severity:** MEDIUM
+
+### 🟡 Upcoming Bills Allows Negative Day Windows — MEDIUM
+**Priority:** MEDIUM  
+**Added:** 2026-05-10 by Prime (code review)  
+**Type:** BUG / API_CONTRACT
+
+**Description:**
+`GET /api/tracker/upcoming` caps `days` at 365 but does not enforce a lower bound or reject non-numeric input. `days=-30` creates a cutoff before today and returns no upcoming bills; `days=abc` leaves `cutoff` invalid. The endpoint contract says upcoming bills in the next N days, so invalid windows should be rejected or normalized.
+
+**Affected Files:**
+- `routes/tracker.js:245-253`
+- `routes/tracker.js:284-291`
+
+**Potential Fix:**
+Parse `days` with integer validation, require `1 <= days <= 365`, and return standardized 400 errors for invalid input.
+
+**Severity:** MEDIUM
+
+
 ### Architecture: Business Logic Mixed with Route Handlers
 **Priority:** MEDIUM  
 **Added:** 2026-05-08 by Neo
@@ -91,6 +286,44 @@ Many routes contain business logic that should be extracted to service layers.
 
 ### 🔵 LOW
 
+
+### 🔵 Export Formats Include Sensitive Bill Credential Fields by Default — LOW
+**Priority:** LOW  
+**Added:** 2026-05-10 by Prime (code review)  
+**Type:** SECURITY / PRIVACY
+
+**Description:**
+Full user exports include `website`, `username`, `account_info`, notes, and monthly notes by default. This may be intended for portability, but it turns every Excel/SQLite export into a high-sensitivity artifact and there is no lightweight/redacted export option.
+
+**Affected Files:**
+- `routes/export.js:88-153`
+- `routes/export.js:156-199`
+- `routes/profile.js:236-254`
+
+**Potential Fix:**
+Add explicit UI copy warning that exports may contain account metadata, and consider a redacted export mode that excludes credential/account fields and free-form notes.
+
+**Severity:** LOW
+
+### 🔵 Duplicate Local Login Route Increases Auth Drift Risk — LOW
+**Priority:** LOW  
+**Added:** 2026-05-10 by Prime (code review)  
+**Type:** TECH_DEBT / SECURITY
+
+**Description:**
+Local username/password login logic exists in both `routes/auth.js` and `routes/authLogin.js`. Only `routes/auth.js` appears mounted in `server.js`, while `authLogin.js` is a near-duplicate public login handler. Duplicate auth code can silently drift on rate limiting, audit logging, CSRF exemptions, and error handling.
+
+**Affected Files:**
+- `routes/auth.js:17-44`
+- `routes/authLogin.js:1-39`
+- `server.js:73-75`
+
+**Potential Fix:**
+Delete the unused route module or refactor both route registrations to call one shared login handler. Add a startup/router inventory test so orphan auth routes do not linger.
+
+**Severity:** LOW
+
+
 ### Add comprehensive unit and integration tests
 **Priority:** LOW  
 **Added:** 2026-05-08 by Scarlett

routes/auth.js

@@ -31,9 +31,10 @@ router.post('/login', (req, res, next) => {
     return res.status(400).json(standardizeError('Username and password are required', 'VALIDATION_ERROR', !username ? 'username' : 'password'));
   }
 
+  try {
     const result = await login(username, password);
     if (!result) {
-    logAudit({ user_id: null, action: 'login.failure', details: { username: req.body.username }, ip_address: req.ip, user_agent: req.get('user-agent') });
+      logAudit({ user_id: null, action: 'login.failure', details: { username }, ip_address: req.ip, user_agent: req.get('user-agent') });
       return res.status(401).json(standardizeError('Invalid username or password', 'AUTH_ERROR'));
     }
 
@@ -41,6 +42,10 @@ router.post('/login', (req, res, next) => {
 
     res.cookie(COOKIE_NAME, result.sessionId, cookieOpts(req));
     res.json({ user: result.user });
+  } catch (err) {
+    console.error('Login error:', err);
+    res.status(500).json(standardizeError('Login failed', 'SERVER_ERROR'));
+  }
 });
 
 // POST /api/auth/logout

routes/authLogin.js

@@ -1,39 +0,0 @@
-const express = require('express');
-const router = express.Router();
-
-const { getSetting } = require('../db/database');
-const { login, cookieOpts, COOKIE_NAME } = require('../services/authService');
-const { logAudit } = require('../services/auditService');
-const { standardizeError } = require('../middleware/errorFormatter');
-
-// POST /api/auth/login
-// Public endpoint - no CSRF protection needed (no session to hijack)
-router.post('/login', async (req, res) => {
-  // Respect admin-configured login method toggle
-  if (getSetting('local_login_enabled') === 'false') {
-    return res.status(403).json(standardizeError('Local username/password login is not enabled on this server.', 'FORBIDDEN'));
-  }
-
-  const { username, password } = req.body;
-  if (!username || !password) {
-    return res.status(400).json(standardizeError('Username and password are required', 'VALIDATION_ERROR', !username ? 'username' : 'password'));
-  }
-
-  try {
-    const result = await login(username, password);
-    if (!result) {
-      logAudit({ user_id: null, action: 'login.failure', details: { username }, ip_address: req.ip, user_agent: req.get('user-agent') });
-      return res.status(401).json(standardizeError('Invalid username or password', 'AUTH_ERROR'));
-    }
-
-    logAudit({ user_id: result.user.id, action: 'login.success', ip_address: req.ip, user_agent: req.get('user-agent') });
-
-    res.cookie(COOKIE_NAME, result.sessionId, cookieOpts(req));
-    res.json({ user: result.user });
-  } catch (err) {
-    console.error('Login error:', err);
-    res.status(500).json(standardizeError('Login failed', 'SERVER_ERROR'));
-  }
-});
-
-module.exports = router;

server.js

@@ -12,7 +12,6 @@ const { importLimiter, exportLimiter, adminActionLimiter, oidcLimiter, loginLimi
   require('./middleware/rateLimiter');
 const { csrfMiddleware, csrfTokenProvider }                     = require('./middleware/csrf');
 
-const authLoginRouter = require('./routes/authLogin');
 const app  = express();
 const PORT = process.env.PORT || 3000;
 const DIST = path.join(__dirname, 'dist');
@@ -64,7 +63,7 @@ function skipRateLimitIfNoUsers(limiter) {
 
 // Mount login router with conditional rate limiting
 // If no users exist, rate limit is bypassed; otherwise it applies
-app.use('/api/auth/login', skipRateLimitIfNoUsers(loginLimiter), authLoginRouter);
+app.use('/api/auth/login', skipRateLimitIfNoUsers(loginLimiter));
 // Password change routes are exempt from CSRF - session-based auth is primary protection
 // Set csrfSkip before the auth routes middleware runs
 app.use('/api/auth/change-password', (req, res, next) => { req.csrfSkip = true; next(); });