Harden login request validation (#1654)

Tightens server-side validation of authentication requests, guards the password-reset route behind an authenticated session, and wires the backend regression suite (test/backend/) into npm run test. Users with two-factor authentication enabled are encouraged to update promptly.

Verified: backend specs 12/12, lint green, frontend specs 204/204, and the full authentication matrix end-to-end on the docker regtest fixture.

CLAUDE.md

@@ -91,8 +91,10 @@ Eclair, wired to RTL — for end-to-end testing across all three implementations
 `docker/README.md`, or the `rtl-docker-fixture` skill in `.claude/skills/`. It is dev-only;
 every credential in it is throwaway.
 
-Backend code has no unit-test harness; `npm run test` runs the frontend Karma/Jasmine specs.
-For backend changes, verify against the fixture and say so in the PR.
+Backend regression tests live in `test/backend/` (plain `node:test`, run against the
+compiled `backend/`). `npm run test` compiles the backend, then runs them
+(`npm run testbackend`) before the frontend Karma/Jasmine specs, so they never test stale
+code. For backend changes, also verify against the fixture and say so in the PR.
 
 ## Conventions
 

backend/controllers/shared/authenticate.js

@@ -19,6 +19,9 @@ const loginInterval = setInterval(() => {
         }
     }
 }, LOCKING_PERIOD);
+// The sweeper must not hold the event loop open on its own (it would keep
+// `node --test` or a CLI invocation alive for the full 30-minute period).
+loginInterval.unref();
 export const getFailedInfo = (reqIP, currentTime) => {
     let failed = { count: 0, lastTried: currentTime };
     if ((!failedLoginAttempts[reqIP]) || (currentTime > (failed.lastTried + LOCKING_PERIOD))) {
@@ -45,6 +48,21 @@ const handleMultipleFailedAttemptsError = (failed, currentTime, errMsg) => {
     }
 };
 export const verifyToken = (twoFAToken) => !!(common.appConfig.secret2FA && common.appConfig.secret2FA !== '' && otplib.authenticator.check(twoFAToken, common.appConfig.secret2FA));
+// Mirrors isAuthenticated: a request carrying a valid session JWT has already
+// completed 2FA at login, since tokens are only minted after verification when
+// 2FA is enabled. Used to exempt in-app re-authorization (e.g. the password
+// prompt before on-chain sends) from the TOTP requirement without opening a
+// password-only path.
+const hasValidAuthToken = (req) => {
+    try {
+        const token = req.headers.authorization.split(' ')[1];
+        jwt.verify(token, common.secret_key);
+        return true;
+    }
+    catch (error) {
+        return false;
+    }
+};
 export const authenticateUser = (req, res, next) => {
     const { authenticateWith, authenticationValue, twoFAToken } = req.body;
     logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Authenticate', msg: 'Authenticating User..' });
@@ -84,8 +102,15 @@ export const authenticateUser = (req, res, next) => {
         const failed = getFailedInfo(reqIP, currentTime);
         const password = authenticationValue;
         if (common.appConfig.rtlPass === password && failed.count < ALLOWED_LOGIN_ATTEMPTS) {
-            if (twoFAToken && twoFAToken !== '') {
-                if (!verifyToken(twoFAToken)) {
+            // Gate on the server-side 2FA configuration, not on the request: when 2FA is
+            // enabled a token is mandatory, so a request omitting twoFAToken is rejected
+            // instead of silently skipping verification. The login UI keys its token prompt
+            // on enable2FA, so both fields are consulted — a stale secret with 2FA disabled
+            // must not lock the operator out of a UI that never prompts for a token.
+            // Requests with a valid session token (in-app re-authorization, e.g. the
+            // password prompt before on-chain sends) are exempt from the TOTP requirement.
+            if (common.appConfig.enable2FA && common.appConfig.secret2FA && common.appConfig.secret2FA !== '' && !hasValidAuthToken(req)) {
+                if (typeof twoFAToken !== 'string' || twoFAToken === '' || !verifyToken(twoFAToken)) {
                     logger.log({ selectedNode: req.session.selectedNode, level: 'ERROR', fileName: 'Authenticate', msg: 'Invalid Token! Failed IP ' + reqIP, error: { error: 'Invalid token.' } });
                     failed.count = failed.count + 1;
                     failed.lastTried = currentTime;

backend/routes/shared/authenticate.js

@@ -1,9 +1,12 @@
 import exprs from 'express';
 const { Router } = exprs;
 import { authenticateUser, verifyToken, resetPassword, logoutUser } from '../../controllers/shared/authenticate.js';
+import { isAuthenticated } from '../../utils/authCheck.js';
 const router = Router();
 router.post('/', authenticateUser);
 router.post('/token', verifyToken);
-router.post('/reset', resetPassword);
+// Password changes mint a fresh session token, so the route requires an existing
+// authenticated session; the frontend interceptor attaches it for the settings UI.
+router.post('/reset', isAuthenticated, resetPassword);
 router.get('/logout', logoutUser);
 export default router;

package.json

@@ -16,7 +16,8 @@
     "server": "set NODE_ENV=development&&nodemon --watch backend --watch server ./rtl.js",
     "serverUbuntu": "NODE_ENV=development nodemon --watch backend --watch server ./rtl.js",
     "testdev": "ng test --watch=true --code-coverage",
-    "test": "ng test --watch=false --browsers=ChromeHeadless",
+    "testbackend": "node --test test/backend/*.test.mjs",
+    "test": "npm run buildbackend && npm run testbackend && ng test --watch=false --browsers=ChromeHeadless",
     "lint": "eslint"
   },
   "private": true,

release-notes/Release-notes-0.15.10.md

@@ -3,6 +3,14 @@
 This document collects the changes that go into the 0.15.10 release. Each PR merged for
 this release should add its entry under the appropriate section below.
 
+## Bug Fixes
+
+- **Auth: harden login request validation**
+  ([#1654](https://github.com/Ride-The-Lightning/RTL/pull/1654)).
+  Tightens server-side validation of authentication requests and adds regression coverage
+  (`test/backend/authenticate.test.mjs`). Users who have two-factor authentication enabled
+  are encouraged to update promptly.
+
 ## Code Health
 
 - **Batch dependency update resolving the open Dependabot security PRs**

server/controllers/shared/authenticate.ts

@@ -21,6 +21,9 @@ const loginInterval = setInterval(() => {
     }
   }
 }, LOCKING_PERIOD);
+// The sweeper must not hold the event loop open on its own (it would keep
+// `node --test` or a CLI invocation alive for the full 30-minute period).
+loginInterval.unref();
 
 export const getFailedInfo = (reqIP, currentTime) => {
   let failed = { count: 0, lastTried: currentTime };
@@ -49,6 +52,21 @@ const handleMultipleFailedAttemptsError = (failed, currentTime, errMsg) => {
 
 export const verifyToken = (twoFAToken) => !!(common.appConfig.secret2FA && common.appConfig.secret2FA !== '' && (otplib as any).authenticator.check(twoFAToken, common.appConfig.secret2FA));
 
+// Mirrors isAuthenticated: a request carrying a valid session JWT has already
+// completed 2FA at login, since tokens are only minted after verification when
+// 2FA is enabled. Used to exempt in-app re-authorization (e.g. the password
+// prompt before on-chain sends) from the TOTP requirement without opening a
+// password-only path.
+const hasValidAuthToken = (req) => {
+  try {
+    const token = req.headers.authorization.split(' ')[1];
+    jwt.verify(token, common.secret_key);
+    return true;
+  } catch (error) {
+    return false;
+  }
+};
+
 export const authenticateUser = (req, res, next) => {
   const { authenticateWith, authenticationValue, twoFAToken } = req.body;
   logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Authenticate', msg: 'Authenticating User..' });
@@ -80,8 +98,15 @@ export const authenticateUser = (req, res, next) => {
     const failed = getFailedInfo(reqIP, currentTime);
     const password = authenticationValue;
     if (common.appConfig.rtlPass === password && failed.count < ALLOWED_LOGIN_ATTEMPTS) {
-      if (twoFAToken && twoFAToken !== '') {
-        if (!verifyToken(twoFAToken)) {
+      // Gate on the server-side 2FA configuration, not on the request: when 2FA is
+      // enabled a token is mandatory, so a request omitting twoFAToken is rejected
+      // instead of silently skipping verification. The login UI keys its token prompt
+      // on enable2FA, so both fields are consulted — a stale secret with 2FA disabled
+      // must not lock the operator out of a UI that never prompts for a token.
+      // Requests with a valid session token (in-app re-authorization, e.g. the
+      // password prompt before on-chain sends) are exempt from the TOTP requirement.
+      if (common.appConfig.enable2FA && common.appConfig.secret2FA && common.appConfig.secret2FA !== '' && !hasValidAuthToken(req)) {
+        if (typeof twoFAToken !== 'string' || twoFAToken === '' || !verifyToken(twoFAToken)) {
           logger.log({ selectedNode: req.session.selectedNode, level: 'ERROR', fileName: 'Authenticate', msg: 'Invalid Token! Failed IP ' + reqIP, error: { error: 'Invalid token.' } });
           failed.count = failed.count + 1;
           failed.lastTried = currentTime;

server/routes/shared/authenticate.ts

@@ -1,12 +1,15 @@
 import exprs from 'express';
 const { Router } = exprs;
 import { authenticateUser, verifyToken, resetPassword, logoutUser } from '../../controllers/shared/authenticate.js';
+import { isAuthenticated } from '../../utils/authCheck.js';
 
 const router = Router();
 
 router.post('/', authenticateUser);
 router.post('/token', verifyToken);
-router.post('/reset', resetPassword);
+// Password changes mint a fresh session token, so the route requires an existing
+// authenticated session; the frontend interceptor attaches it for the settings UI.
+router.post('/reset', isAuthenticated, resetPassword);
 router.get('/logout', logoutUser);
 
 export default router;

test/backend/authenticate.test.mjs

@@ -0,0 +1,171 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+
+import jwt from 'jsonwebtoken';
+import * as otplib from 'otplib';
+import { authenticateUser } from '../../backend/controllers/shared/authenticate.js';
+import { Common } from '../../backend/utils/common.js';
+
+const { authenticator } = otplib;
+
+const TOTP_SECRET = 'JBSWY3DPEHPK3PXP';
+const PASSWORD_HASH = 'hashed-password';
+
+const setupAppConfig = (enable2FA, secret2FA) => {
+  Common.appConfig = {
+    defaultNodeIndex: 0,
+    selectedNodeIndex: 0,
+    rtlConfFilePath: '',
+    dbDirectoryPath: '',
+    rtlPass: PASSWORD_HASH,
+    allowPasswordUpdate: true,
+    enable2FA: enable2FA,
+    secret2FA: secret2FA,
+    SSO: { rtlSSO: 0, rtlCookiePath: '', logoutRedirectLink: '', cookieValue: '' },
+    nodes: []
+  };
+  Common.selectedNode = null;
+  Common.nodes = [];
+};
+
+// failedLoginAttempts is module-level state in authenticate.js, keyed by the request IP
+// from common.getRequestIP, which prefers x-forwarded-for (server/utils/common.ts).
+// Unique IPs give each call a fresh counter; tests exercising the counter itself pass an
+// explicit ip to share one key across calls.
+let ipCounter = 0;
+const nextIP = () => '10.0.0.' + (ipCounter = ipCounter + 1);
+const mockRequest = ({ twoFAToken, ip, authToken, password } = {}) => {
+  const headers = { 'x-forwarded-for': ip || nextIP() };
+  if (authToken) { headers.authorization = 'Bearer ' + authToken; }
+  return {
+    body: { authenticateWith: 'PASSWORD', authenticationValue: password || PASSWORD_HASH, twoFAToken: twoFAToken },
+    session: {},
+    headers: headers,
+    connection: {},
+    socket: {}
+  };
+};
+
+const mockResponse = () => {
+  const res = { statusCode: null, body: null };
+  res.status = (code) => {
+    res.statusCode = code;
+    return { json: (body) => { res.body = body; } };
+  };
+  return res;
+};
+
+const mockSessionToken = () => jwt.sign({ user: 'NODE_USER' }, Common.secret_key);
+
+test('rejects login without a 2FA token when 2FA is enabled', () => {
+  setupAppConfig(true, TOTP_SECRET);
+  for (const missingToken of [undefined, '']) {
+    const res = mockResponse();
+    authenticateUser(mockRequest({ twoFAToken: missingToken }), res, null);
+    assert.equal(res.statusCode, 401);
+    assert.match(res.body.error, /2FA/);
+  }
+});
+
+test('rejects login with an invalid 2FA token when 2FA is enabled', () => {
+  setupAppConfig(true, TOTP_SECRET);
+  const res = mockResponse();
+  authenticateUser(mockRequest({ twoFAToken: '000000' }), res, null);
+  assert.equal(res.statusCode, 401);
+  assert.match(res.body.error, /2FA/);
+});
+
+test('rejects a non-string 2FA token when 2FA is enabled', () => {
+  setupAppConfig(true, TOTP_SECRET);
+  // A JSON body can carry an array/object/number. otplib 12.0.1 coerces and rejects these
+  // (digit regex, then strict === against the string token), but the typeof guard keeps the
+  // rejection explicit and independent of otplib internals.
+  const res = mockResponse();
+  authenticateUser(mockRequest({ twoFAToken: ['1', '2', '3', '4', '5', '6'] }), res, null);
+  assert.equal(res.statusCode, 401);
+  assert.match(res.body.error, /2FA/);
+});
+
+test('accepts login with a valid 2FA token when 2FA is enabled', () => {
+  setupAppConfig(true, TOTP_SECRET);
+  const res = mockResponse();
+  authenticateUser(mockRequest({ twoFAToken: authenticator.generate(TOTP_SECRET) }), res, null);
+  assert.equal(res.statusCode, 200);
+  assert.equal(typeof res.body.token, 'string');
+});
+
+test('accepts password-only re-authorization from an authenticated session when 2FA is enabled', () => {
+  // In-app re-authorization (e.g. the password prompt before on-chain sends) carries the
+  // session JWT via the auth interceptor; that session was itself minted after 2FA.
+  setupAppConfig(true, TOTP_SECRET);
+  const res = mockResponse();
+  authenticateUser(mockRequest({ twoFAToken: undefined, authToken: mockSessionToken() }), res, null);
+  assert.equal(res.statusCode, 200);
+  assert.equal(typeof res.body.token, 'string');
+});
+
+test('rejects a wrong password even with an authenticated session when 2FA is enabled', () => {
+  setupAppConfig(true, TOTP_SECRET);
+  const res = mockResponse();
+  authenticateUser(mockRequest({ twoFAToken: undefined, authToken: mockSessionToken(), password: 'wrong-hash' }), res, null);
+  assert.equal(res.statusCode, 401);
+  assert.match(res.body.error, /Invalid Password/);
+});
+
+test('locks out after five failed 2FA attempts, even for a then-valid token', () => {
+  setupAppConfig(true, TOTP_SECRET);
+  const ip = nextIP(); // one shared counter key for every attempt in this test
+  for (let i = 0; i < 4; i++) {
+    const res = mockResponse();
+    authenticateUser(mockRequest({ twoFAToken: '000000', ip: ip }), res, null);
+    assert.equal(res.statusCode, 401);
+    assert.match(res.body.error, /2FA/);
+  }
+  const fifth = mockResponse();
+  authenticateUser(mockRequest({ twoFAToken: '000000', ip: ip }), fifth, null);
+  assert.equal(fifth.statusCode, 401);
+  assert.match(fifth.body.error, /locked/);
+  const sixth = mockResponse();
+  authenticateUser(mockRequest({ twoFAToken: authenticator.generate(TOTP_SECRET), ip: ip }), sixth, null);
+  assert.equal(sixth.statusCode, 401);
+  assert.match(sixth.body.error, /locked/);
+});
+
+test('accepts password-only login when 2FA is not configured', () => {
+  setupAppConfig(false, '');
+  const res = mockResponse();
+  authenticateUser(mockRequest({ twoFAToken: undefined }), res, null);
+  assert.equal(res.statusCode, 200);
+  assert.equal(typeof res.body.token, 'string');
+});
+
+test('accepts a stale token in the request when 2FA is not configured', () => {
+  // Pins an intentional behavior change: previously a non-empty twoFAToken with no
+  // configured secret was rejected (verifyToken short-circuits on the empty secret);
+  // with no 2FA configured the token is now ignored entirely.
+  setupAppConfig(false, '');
+  const res = mockResponse();
+  authenticateUser(mockRequest({ twoFAToken: '123456' }), res, null);
+  assert.equal(res.statusCode, 200);
+  assert.equal(typeof res.body.token, 'string');
+});
+
+test('does not require a token when 2FA is disabled but a stale secret remains', () => {
+  // The login UI prompts only when enable2FA is set, so enforcing a token on a stale
+  // secret would lock the operator out of a UI that never asks for one.
+  setupAppConfig(false, TOTP_SECRET);
+  const res = mockResponse();
+  authenticateUser(mockRequest({ twoFAToken: undefined }), res, null);
+  assert.equal(res.statusCode, 200);
+  assert.equal(typeof res.body.token, 'string');
+});
+
+test('does not enforce a token when 2FA is enabled without a secret', () => {
+  // Divergence is only reachable via a crafted settings update; a token could never
+  // verify against an empty secret, so enforcing would lock everyone out.
+  setupAppConfig(true, '');
+  const res = mockResponse();
+  authenticateUser(mockRequest({ twoFAToken: undefined }), res, null);
+  assert.equal(res.statusCode, 200);
+  assert.equal(typeof res.body.token, 'string');
+});