Harden login request validation

backend/controllers/shared/authenticate.js

@@ -84,8 +84,11 @@ 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 a secret is
+            // configured a token is mandatory, so a request omitting twoFAToken is rejected
+            // instead of silently skipping verification.
+            if (common.appConfig.secret2FA && common.appConfig.secret2FA !== '') {
+                if (!twoFAToken || 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;

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**
+  ([#TBD](https://github.com/Ride-The-Lightning/RTL/pull/TBD)).
+  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

@@ -80,8 +80,11 @@ 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 a secret is
+      // configured a token is mandatory, so a request omitting twoFAToken is rejected
+      // instead of silently skipping verification.
+      if (common.appConfig.secret2FA && common.appConfig.secret2FA !== '') {
+        if (!twoFAToken || 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;

test/backend/authenticate.test.mjs

@@ -0,0 +1,85 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+
+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 = (secret2FA) => {
+  Common.appConfig = {
+    defaultNodeIndex: 0,
+    selectedNodeIndex: 0,
+    rtlConfFilePath: '',
+    dbDirectoryPath: '',
+    rtlPass: PASSWORD_HASH,
+    allowPasswordUpdate: true,
+    enable2FA: !!secret2FA,
+    secret2FA: secret2FA,
+    SSO: { rtlSSO: 0, rtlCookiePath: '', logoutRedirectLink: '', cookieValue: '' },
+    nodes: []
+  };
+  Common.selectedNode = null;
+  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.
+let ipCounter = 0;
+const mockRequest = (twoFAToken) => {
+  ipCounter = ipCounter + 1;
+  return {
+    body: { authenticateWith: 'PASSWORD', authenticationValue: PASSWORD_HASH, twoFAToken: twoFAToken },
+    session: {},
+    headers: { 'x-forwarded-for': '10.0.0.' + ipCounter },
+    connection: {},
+    socket: {}
+  };
+};
+
+const mockResponse = () => {
+  const res = { statusCode: null, body: null };
+  res.status = (code) => {
+    res.statusCode = code;
+    return { json: (body) => { res.body = body; } };
+  };
+  return res;
+};
+
+test('rejects login without a 2FA token when 2FA is enabled', () => {
+  setupAppConfig(TOTP_SECRET);
+  for (const missingToken of [undefined, '']) {
+    const res = mockResponse();
+    authenticateUser(mockRequest(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(TOTP_SECRET);
+  const res = mockResponse();
+  authenticateUser(mockRequest('000000'), 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(TOTP_SECRET);
+  const res = mockResponse();
+  authenticateUser(mockRequest(authenticator.generate(TOTP_SECRET)), res, null);
+  assert.equal(res.statusCode, 200);
+  assert.equal(typeof res.body.token, 'string');
+});
+
+test('accepts password-only login when 2FA is not configured', () => {
+  setupAppConfig('');
+  const res = mockResponse();
+  authenticateUser(mockRequest(undefined), res, null);
+  assert.equal(res.statusCode, 200);
+  assert.equal(typeof res.body.token, 'string');
+});