Address review findings on auth session handling and test wiring

CLAUDE.md

@@ -92,9 +92,9 @@ Eclair, wired to RTL — for end-to-end testing across all three implementations
 every credential in it is throwaway.
 
 Backend regression tests live in `test/backend/` (plain `node:test`, run against the
-compiled `backend/` — run `npm run buildbackend` first or they test stale code).
-`npm run test` chains them (`npm run testbackend`) before the frontend Karma/Jasmine specs.
-For backend changes, also verify against the fixture and say so in the PR.
+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

@@ -48,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..' });
@@ -92,7 +107,9 @@ export const authenticateUser = (req, res, next) => {
             // 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.
-            if (common.appConfig.enable2FA && common.appConfig.secret2FA && common.appConfig.secret2FA !== '') {
+            // 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;

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,8 +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",
-    "testbackend": "node --test \"test/backend/*.test.mjs\"",
-    "test": "npm run testbackend && 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,

server/controllers/shared/authenticate.ts

@@ -52,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..' });
@@ -88,7 +103,9 @@ export const authenticateUser = (req, res, next) => {
       // 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.
-      if (common.appConfig.enable2FA && common.appConfig.secret2FA && common.appConfig.secret2FA !== '') {
+      // 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;

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

@@ -1,6 +1,7 @@
 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';
@@ -27,15 +28,19 @@ const setupAppConfig = (enable2FA, secret2FA) => {
   Common.nodes = [];
 };
 
-// Distinct IPs per call: failedLoginAttempts is module-level state in authenticate.js,
-// keyed by request IP, so unique values keep the tests isolated from each other.
+// 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 mockRequest = (twoFAToken) => {
-  ipCounter = ipCounter + 1;
+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_HASH, twoFAToken: twoFAToken },
+    body: { authenticateWith: 'PASSWORD', authenticationValue: password || PASSWORD_HASH, twoFAToken: twoFAToken },
     session: {},
-    headers: { 'x-forwarded-for': '10.0.0.' + ipCounter },
+    headers: headers,
     connection: {},
     socket: {}
   };
@@ -50,11 +55,13 @@ const mockResponse = () => {
   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(missingToken), res, null);
+    authenticateUser(mockRequest({ twoFAToken: missingToken }), res, null);
     assert.equal(res.statusCode, 401);
     assert.match(res.body.error, /2FA/);
   }
@@ -63,7 +70,7 @@ test('rejects login without a 2FA token when 2FA is enabled', () => {
 test('rejects login with an invalid 2FA token when 2FA is enabled', () => {
   setupAppConfig(true, TOTP_SECRET);
   const res = mockResponse();
-  authenticateUser(mockRequest('000000'), res, null);
+  authenticateUser(mockRequest({ twoFAToken: '000000' }), res, null);
   assert.equal(res.statusCode, 401);
   assert.match(res.body.error, /2FA/);
 });
@@ -74,7 +81,7 @@ test('rejects a non-string 2FA token when 2FA is enabled', () => {
   // (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(['1', '2', '3', '4', '5', '6']), res, null);
+  authenticateUser(mockRequest({ twoFAToken: ['1', '2', '3', '4', '5', '6'] }), res, null);
   assert.equal(res.statusCode, 401);
   assert.match(res.body.error, /2FA/);
 });
@@ -82,15 +89,52 @@ test('rejects a non-string 2FA token when 2FA is enabled', () => {
 test('accepts login with a valid 2FA token when 2FA is enabled', () => {
   setupAppConfig(true, TOTP_SECRET);
   const res = mockResponse();
-  authenticateUser(mockRequest(authenticator.generate(TOTP_SECRET)), res, null);
+  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(undefined), res, null);
+  authenticateUser(mockRequest({ twoFAToken: undefined }), res, null);
   assert.equal(res.statusCode, 200);
   assert.equal(typeof res.body.token, 'string');
 });
@@ -101,7 +145,7 @@ test('accepts a stale token in the request when 2FA is not configured', () => {
   // with no 2FA configured the token is now ignored entirely.
   setupAppConfig(false, '');
   const res = mockResponse();
-  authenticateUser(mockRequest('123456'), res, null);
+  authenticateUser(mockRequest({ twoFAToken: '123456' }), res, null);
   assert.equal(res.statusCode, 200);
   assert.equal(typeof res.body.token, 'string');
 });
@@ -111,7 +155,7 @@ test('does not require a token when 2FA is disabled but a stale secret remains',
   // secret would lock the operator out of a UI that never asks for one.
   setupAppConfig(false, TOTP_SECRET);
   const res = mockResponse();
-  authenticateUser(mockRequest(undefined), res, null);
+  authenticateUser(mockRequest({ twoFAToken: undefined }), res, null);
   assert.equal(res.statusCode, 200);
   assert.equal(typeof res.body.token, 'string');
 });
@@ -121,7 +165,7 @@ test('does not enforce a token when 2FA is enabled without a secret', () => {
   // verify against an empty secret, so enforcing would lock everyone out.
   setupAppConfig(true, '');
   const res = mockResponse();
-  authenticateUser(mockRequest(undefined), res, null);
+  authenticateUser(mockRequest({ twoFAToken: undefined }), res, null);
   assert.equal(res.statusCode, 200);
   assert.equal(typeof res.body.token, 'string');
 });