From 706c6447eb511dfd9967d3eed168f3cefbdddc90 Mon Sep 17 00:00:00 2001 From: saubyk <39208279+saubyk@users.noreply.github.com> Date: Mon, 3 Aug 2026 09:17:31 -0700 Subject: [PATCH] Harden login request validation --- backend/controllers/shared/authenticate.js | 7 +- release-notes/Release-notes-0.15.10.md | 8 ++ server/controllers/shared/authenticate.ts | 7 +- test/backend/authenticate.test.mjs | 85 ++++++++++++++++++++++ 4 files changed, 103 insertions(+), 4 deletions(-) create mode 100644 test/backend/authenticate.test.mjs diff --git a/backend/controllers/shared/authenticate.js b/backend/controllers/shared/authenticate.js index 62b96ec3..91e28b6b 100644 --- a/backend/controllers/shared/authenticate.js +++ b/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; diff --git a/release-notes/Release-notes-0.15.10.md b/release-notes/Release-notes-0.15.10.md index efafa0a1..0c501701 100644 --- a/release-notes/Release-notes-0.15.10.md +++ b/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** diff --git a/server/controllers/shared/authenticate.ts b/server/controllers/shared/authenticate.ts index 252f6b88..09e821b2 100644 --- a/server/controllers/shared/authenticate.ts +++ b/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; diff --git a/test/backend/authenticate.test.mjs b/test/backend/authenticate.test.mjs new file mode 100644 index 00000000..a2a748fc --- /dev/null +++ b/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'); +});