diff --git a/.github/workflows/pr-tests.yml b/.github/workflows/pr-tests.yml index 3ee322d..2255662 100644 --- a/.github/workflows/pr-tests.yml +++ b/.github/workflows/pr-tests.yml @@ -136,6 +136,10 @@ jobs: # directory layout (dc=example,dc=com) -- only the admin password # (normally supplied via a gitignored secrets.js) needs setting. app_ldap__bindPassword: your-ldap-password + # routes/oauth.js now refuses to start without a real jwtSecret. + # This is a non-secret test value; the container under test uses + # secrets.js.example's jwtSecret independently. + app_oauth__jwtSecret: ci-test-jwt-secret-do-not-use-in-production run: npm test test-summary: diff --git a/CHANGELOG.md b/CHANGELOG.md index a227e82..3615fac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,23 @@ correspond to git tags (`vX.Y.Z`) and `nodejs/package.json`'s `version`. ## [Unreleased] +## [1.1.16] - 2026-07-18 + +### Security +- Hardened LDAP filter and DN construction against injection. All user-supplied values interpolated into group filters (`models/group_ldap.js`) and RDN values used when adding users/groups (`models/user_ldap.js`) are now escaped before being sent to the LDAP server. +- Replaced `Math.random()`-based token generation in `models/token.js`, `models/oauth_code.js`, and `models/oauth_client.js` with `crypto.randomUUID()` for session tokens, OAuth codes, access/refresh tokens, and client IDs. +- Replaced `Math.random()`-based OTP generation in `OtpToken.issue()` with `crypto.randomInt()`. +- `routes/oauth.js` now refuses to start if `oauth.jwtSecret` is missing or still set to the placeholder value, instead of falling back to a hardcoded public string. +- Rendered docs and Terms-of-Service HTML in `routes/docs.js` and `routes/index.js` are now sanitized with `xss` to prevent stored XSS from malicious markdown. +- Removed a `console.log` that wrote new-user data (including password hashes) to the log in `models/user_ldap.js`; reduced login-path error logging to `error.name`/`error.message` only. + +### Changed +- Public-release packaging: removed `"private": true` from `nodejs/package.json` and bumped version to `1.1.16`. +- CI workflow (`.github/workflows/pr-tests.yml`) now sets `app_oauth__jwtSecret` so the test suite can run against the new startup-time JWT validation. + +### Fixed +- `models/email.js`: fixed a template bug where the rendered `from` address used `template.message` instead of `template.from`. + ## [1.1.15] - 2026-07-18 ### Changed @@ -116,7 +133,7 @@ First tagged release. Establishes the `vX.Y.Z` tag convention that the in-app up - Unix/POSIX and LDAP bind-only service account support, distinct from real-person accounts. - Merged OAuth Apps + LDAP Info into a single Integrations page. -[Unreleased]: https://github.com/theta42/sso-manager-node/compare/v1.1.15...HEAD +[Unreleased]: https://github.com/theta42/sso-manager-node/compare/v1.1.16...HEAD [1.1.15]: https://github.com/theta42/sso-manager-node/compare/v1.1.14...v1.1.15 [1.1.14]: https://github.com/theta42/sso-manager-node/compare/v1.1.13...v1.1.14 [1.1.13]: https://github.com/theta42/sso-manager-node/compare/v1.1.12...v1.1.13 diff --git a/nodejs/models/auth.js b/nodejs/models/auth.js index 85d69f0..21aebbf 100644 --- a/nodejs/models/auth.js +++ b/nodejs/models/auth.js @@ -23,7 +23,7 @@ Auth.login = async function(data){ return {user, token} }catch(error){ - console.error("AUTH LOGIN error:", error); + console.error("AUTH LOGIN error:", error.name, error.message); throw this.errors.login(); } }; diff --git a/nodejs/models/email.js b/nodejs/models/email.js index b131715..80bb7ee 100644 --- a/nodejs/models/email.js +++ b/nodejs/models/email.js @@ -58,7 +58,7 @@ Mail.sendTemplate = async function(to, template, context, from){ to, mustache.render(template.subject, context), mustache.render(template.message, context), - from || (template.from && mustache.render(template.message, context)) + from || (template.from && mustache.render(template.from, context)) ) }; diff --git a/nodejs/models/group_ldap.js b/nodejs/models/group_ldap.js index 43ada8a..4f939aa 100644 --- a/nodejs/models/group_ldap.js +++ b/nodejs/models/group_ldap.js @@ -4,6 +4,31 @@ const { Client, Attribute, Change } = require('ldapts'); const { LRUCache } = require('lru-cache'); const conf = require('@simpleworkjs/conf').ldap; +// Escape a value used inside an LDAP search filter (RFC 4515). +function escapeLDAPSearchValue(val) { + return String(val) + .replace(/\\/g, '\\5c') + .replace(/\*/g, '\\2a') + .replace(/\(/g, '\\28') + .replace(/\)/g, '\\29') + .replace(/\0/g, '\\00'); +} + +// Escape a value used in an LDAP DN (RFC 4514). Defensive: usernames/cns +// are normally alphanumeric, but this prevents metacharacter injection. +function escapeLDAPDNValue(val) { + return String(val) + .replace(/\\/g, '\\\\') + .replace(/,/g, '\\,') + .replace(/\+/g, '\\+') + .replace(/"/g, '\\"') + .replace(//g, '\\>') + .replace(/;/g, '\\;') + .replace(/=/g, '\\=') + .replace(/^\s|\s$/g, match => match === ' ' ? '\\ ' : match); +} + function makeClient() { return new Client({ url: conf.url }); } @@ -19,7 +44,7 @@ async function withClient(fn) { } async function getGroups(client, member){ - let memberFilter = member ? `(member=${member})`: '' + let memberFilter = member ? `(member=${escapeLDAPSearchValue(member)})`: '' let groups = (await client.search(conf.groupBase, { scope: 'sub', @@ -35,7 +60,8 @@ async function getGroups(client, member){ } async function addGroup(client, data){ - await client.add(`cn=${data.name},${conf.groupBase}`, { + const safeName = escapeLDAPDNValue(data.name); + await client.add(`cn=${safeName},${conf.groupBase}`, { cn: data.name, member: data.owner, description: data.description, @@ -139,9 +165,10 @@ Group.get = async function(data){ } return withClient(async (client) => { + const safeName = escapeLDAPSearchValue(data.name); let group = (await client.search(conf.groupBase, { scope: 'sub', - filter: `(&(objectClass=groupOfNames)(cn=${data.name}))`, + filter: `(&(objectClass=groupOfNames)(cn=${safeName}))`, attributes: ['cn', 'description', 'member', 'owner', 'createTimestamp', 'modifyTimestamp'], })).searchEntries[0]; diff --git a/nodejs/models/oauth_client.js b/nodejs/models/oauth_client.js index cea7bce..14063d6 100644 --- a/nodejs/models/oauth_client.js +++ b/nodejs/models/oauth_client.js @@ -2,7 +2,8 @@ const Table = require('.'); const bcrypt = require('bcrypt'); -const UUID = function b(a){return a?(a^Math.random()*16>>a/4).toString(16):([1e7]+-1e3+-4e3+-8e3+-1e11).replace(/[018]/g,b)}; +const crypto = require('crypto'); +const UUID = () => crypto.randomUUID(); const conf = require('@simpleworkjs/conf'); const defaultLifetime = (conf.oauth && conf.oauth.token_lifetime) || { diff --git a/nodejs/models/oauth_code.js b/nodejs/models/oauth_code.js index 7da5665..64739b9 100644 --- a/nodejs/models/oauth_code.js +++ b/nodejs/models/oauth_code.js @@ -1,7 +1,8 @@ 'use strict'; const Table = require('.'); -const UUID = function b(a){return a?(a^Math.random()*16>>a/4).toString(16):([1e7]+-1e3+-4e3+-8e3+-1e11).replace(/[018]/g,b)}; +const crypto = require('crypto'); +const UUID = () => crypto.randomUUID(); // Shared base keyMap matching Token's schema so these behave as tokens const tokenKeyMap = { diff --git a/nodejs/models/token.js b/nodejs/models/token.js index b845783..a282104 100644 --- a/nodejs/models/token.js +++ b/nodejs/models/token.js @@ -1,7 +1,8 @@ 'use strict'; const Table = require('.'); -const UUID = function b(a){return a?(a^Math.random()*16>>a/4).toString(16):([1e7]+-1e3+-4e3+-8e3+-1e11).replace(/[018]/g,b)}; +const crypto = require('crypto'); +const UUID = () => crypto.randomUUID(); class Token extends Table{ @@ -110,7 +111,7 @@ class OtpToken extends Token { for (const t of existing) { if (t.is_valid) await t.update({is_valid: false}); } - const code = String(Math.floor(100000 + Math.random() * 900000)); + const code = String(crypto.randomInt(100000, 1000000)); return this.create({uid, code, method, created_by: uid}); } diff --git a/nodejs/models/user_ldap.js b/nodejs/models/user_ldap.js index 50f3e30..30fe669 100644 --- a/nodejs/models/user_ldap.js +++ b/nodejs/models/user_ldap.js @@ -45,6 +45,20 @@ function escapeLDAPSearchValue(val) { .replace(/\0/g, '\\00'); } +// Escape a value used in an LDAP DN (RFC 4514). +function escapeLDAPDNValue(val) { + return String(val) + .replace(/\\/g, '\\\\') + .replace(/,/g, '\\,') + .replace(/\+/g, '\\+') + .replace(/"/g, '\\"') + .replace(//g, '\\>') + .replace(/;/g, '\\;') + .replace(/=/g, '\\=') + .replace(/^\s|\s$/g, match => match === ' ' ? '\\ ' : match); +} + // Compute the next available uid/gidNumber: the highest existing value below // conf.uidGidReservedFloor, plus one -- or conf.uidGidMin if there are no // such entries yet. Entries at/above the reserved floor (e.g. a bootstrap @@ -72,7 +86,8 @@ async function addPosixGroup(client, data){ data.gidNumber = nextPosixId(groups, 'gidNumber'); - await client.add(`cn=${data.cn},${conf.groupBase}`, { + const safeCn = escapeLDAPDNValue(data.cn); + await client.add(`cn=${safeCn},${conf.groupBase}`, { cn: data.cn, gidNumber: data.gidNumber, objectclass: [ 'posixGroup', 'top' ] @@ -94,6 +109,7 @@ async function addPosixAccount(client, data){ data.uidNumber = nextPosixId(people, 'uidNumber'); + const safeCn = escapeLDAPDNValue(data.cn); const entry = { cn: data.cn, sn: data.sn, @@ -143,7 +159,7 @@ async function addPosixAccount(client, data){ entry.manager = [].concat(data.manager); } - await client.add(`cn=${data.cn},${conf.userBase}`, entry); + await client.add(`cn=${safeCn},${conf.userBase}`, entry); return data @@ -171,7 +187,6 @@ async function addLdapUser(client, data){ delete data.userPassword; } - console.log('addLdapUser', data) group = await addPosixGroup(client, data); data = await addPosixAccount(client, group); @@ -799,7 +814,7 @@ User.addSSHkey = async function(data) { // memberUid (RFC 2307, posixGroup) is a bare username, not a DN, unlike // groupOfNames' `member` used by app_sso_* groups in group_ldap.js. function personalGroupDN(uid){ - return `cn=${uid},${conf.groupBase}`; + return `cn=${escapeLDAPDNValue(uid)},${conf.groupBase}`; } User.getPersonalGroupMembers = async function(uid) { @@ -873,7 +888,7 @@ User.login = async function(data){ return user; }catch(error){ - console.error("USER LOGIN error:", error); + console.error("USER LOGIN error:", error.name, error.message); throw error; } }; diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index 7909c5b..566ca01 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -1,17 +1,17 @@ { "name": "t42-sso-manager", - "version": "1.1.15", + "version": "1.1.16", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "t42-sso-manager", - "version": "1.1.15", + "version": "1.1.16", "license": "MIT", "dependencies": { "@fortawesome/fontawesome-free": "^7.3.0", "@popperjs/core": "^2.11.8", - "@simpleworkjs/conf": "^1.1.0", + "@simpleworkjs/conf": "^1.2.0", "bcrypt": "^6.0.0", "bootstrap": "^5.3.8", "compression": "^1.8.1", @@ -19,7 +19,7 @@ "express": "^5.2.1", "express-rate-limit": "^8.5.2", "extend": "^3.0.2", - "jq-repeat": "^2.1.0", + "jq-repeat": "^2.2.0", "jquery": "^3.7.1", "jsonwebtoken": "^9.0.3", "ldapts": "^8.1.2", @@ -30,7 +30,8 @@ "mustache": "^4.2.0", "nodemailer": "^9.0.0", "p2psub": "^0.2.0", - "socket.io": "^4.8.3" + "socket.io": "^4.8.3", + "xss": "^1.0.15" }, "devDependencies": { "jest": "^30.4.2", @@ -2331,6 +2332,12 @@ "node": ">= 0.8" } }, + "node_modules/commander": { + "version": "2.20.3", + "resolved": "https://registry.npmjs.org/commander/-/commander-2.20.3.tgz", + "integrity": "sha512-GpVkmM8vF2vQUkj2LvZmD35JxeJOLCwJ9cUkugyk2nuhbv3+mJvpLYYt+0+USMxE+oj+ey/lJEnhZw75x/OMcQ==", + "license": "MIT" + }, "node_modules/component-emitter": { "version": "1.3.1", "resolved": "https://registry.npmjs.org/component-emitter/-/component-emitter-1.3.1.tgz", @@ -2488,6 +2495,12 @@ "node": ">= 8" } }, + "node_modules/cssfilter": { + "version": "0.0.10", + "resolved": "https://registry.npmjs.org/cssfilter/-/cssfilter-0.0.10.tgz", + "integrity": "sha512-FAaLDaplstoRsDR8XGYH51znUN0UY7nMc6Z9/fvE8EXGwvJE9hu7W2vHwx1+bd6gCYnln9nLbzxFTrcO9YQDZw==", + "license": "MIT" + }, "node_modules/debug": { "version": "4.4.3", "resolved": "https://registry.npmjs.org/debug/-/debug-4.4.3.tgz", @@ -6488,6 +6501,22 @@ } } }, + "node_modules/xss": { + "version": "1.0.15", + "resolved": "https://registry.npmjs.org/xss/-/xss-1.0.15.tgz", + "integrity": "sha512-FVdlVVC67WOIPvfOwhoMETV72f6GbW7aOabBC3WxN/oUdoEMDyLz4OgRv5/gck2ZeNqEQu+Tb0kloovXOfpYVg==", + "license": "MIT", + "dependencies": { + "commander": "^2.20.3", + "cssfilter": "0.0.10" + }, + "bin": { + "xss": "bin/xss" + }, + "engines": { + "node": ">= 0.10.0" + } + }, "node_modules/y18n": { "version": "5.0.8", "resolved": "https://registry.npmjs.org/y18n/-/y18n-5.0.8.tgz", diff --git a/nodejs/package.json b/nodejs/package.json index 3ea313e..b84b699 100755 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -1,7 +1,6 @@ { "name": "t42-sso-manager", - "version": "1.1.15", - "private": true, + "version": "1.1.16", "author": [ { "name": "William Mantly", @@ -42,7 +41,8 @@ "mustache": "^4.2.0", "nodemailer": "^9.0.0", "p2psub": "^0.2.0", - "socket.io": "^4.8.3" + "socket.io": "^4.8.3", + "xss": "^1.0.15" }, "license": "MIT", "repository": { diff --git a/nodejs/routes/docs.js b/nodejs/routes/docs.js index b6e5bb2..1c6ef1b 100644 --- a/nodejs/routes/docs.js +++ b/nodejs/routes/docs.js @@ -4,6 +4,7 @@ const fs = require('fs'); const path = require('path'); const router = require('express').Router(); const {marked} = require('marked'); +const xss = require('xss'); const conf = require('@simpleworkjs/conf'); const buildInfo = require('../utils/build_info'); const rateLimit = require('../middleware/rate_limit'); @@ -131,7 +132,7 @@ router.get('/:slug', function(req, res, next) { docs: docList, currentSlug: req.params.slug, docTitle: doc.title, - docHtml: fixDocLinks(fixImagePaths(marked(content))), + docHtml: xss(fixDocLinks(fixImagePaths(marked(content)))), }); } catch (error) { next(error); diff --git a/nodejs/routes/index.js b/nodejs/routes/index.js index d389511..d00ffce 100755 --- a/nodejs/routes/index.js +++ b/nodejs/routes/index.js @@ -5,6 +5,7 @@ var express = require('express'); var router = express.Router(); const moment = require('moment'); const {marked} = require('marked'); +const xss = require('xss'); const {InviteToken, PasswordResetToken} = require('./../models/token'); const {Tos} = require('../models/tos'); const conf = require('@simpleworkjs/conf'); @@ -46,7 +47,7 @@ router.get('/health', function(req, res) { router.get('/tos', async function(req, res, next) { try { const tos = await Tos.getCurrent(); - res.render('tos', {...values, tosHtml: marked(tos.content), tosUpdatedOnFmt: moment(tos.updated_on, 'x').format('MMMM YYYY')}); + res.render('tos', {...values, tosHtml: xss(marked(tos.content)), tosUpdatedOnFmt: moment(tos.updated_on, 'x').format('MMMM YYYY')}); } catch (error) { next(error); } @@ -68,7 +69,7 @@ router.get('/invites', function(req, res) { router.get('/onboarding', async function(req, res, next) { try { const tos = await Tos.getCurrent(); - res.render('onboarding', {...values, tosHtml: marked(tos.content)}); + res.render('onboarding', {...values, tosHtml: xss(marked(tos.content))}); } catch (error) { next(error); } diff --git a/nodejs/routes/oauth.js b/nodejs/routes/oauth.js index 8475384..47630cd 100644 Binary files a/nodejs/routes/oauth.js and b/nodejs/routes/oauth.js differ