diff --git a/DEPLOYMENT.md b/DEPLOYMENT.md index 53e1702..ecf7c1c 100644 --- a/DEPLOYMENT.md +++ b/DEPLOYMENT.md @@ -23,6 +23,7 @@ as raw strings otherwise. Examples: |---------|------|------| | `app_ldap__url=ldap://host:389` | `conf.ldap.url` | string | | `app_ldap__bindPassword=secret` | `conf.ldap.bindPassword` | string | +| `app_ldap__uidGidMin=1500` | `conf.ldap.uidGidMin` | number (new-user id floor) | | `app_oauth__jwtSecret=...` | `conf.oauth.jwtSecret` | string | | `app_smtp__secure=false` | `conf.smtp.secure` | boolean | | `app_oauth__token_lifetime__access_token=3600` | `conf.oauth.token_lifetime.access_token` | number | diff --git a/docs/configuration.md b/docs/configuration.md index 977068b..15a6765 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -29,6 +29,8 @@ raw strings otherwise. | `app_ldap__url=ldap://host:389` | `conf.ldap.url` | string | | `app_ldap__bindPassword=secret` | `conf.ldap.bindPassword` | string | | `app_ldap__userBase=ou=people,dc=…` | `conf.ldap.userBase` | string | +| `app_ldap__uidGidMin=1500` | `conf.ldap.uidGidMin` | number (new-user id floor) | +| `app_ldap__uidGidReservedFloor=9000` | `conf.ldap.uidGidReservedFloor` | number (ids at/above this are ignored when allocating) | | `app_oauth__jwtSecret=...` | `conf.oauth.jwtSecret` | string | | `app_oauth__issuer=https://sso.example.com` | `conf.oauth.issuer` | string | | `app_oauth__token_lifetime__access_token=3600` | `conf.oauth.token_lifetime.access_token` | number | diff --git a/nodejs/conf/base.js b/nodejs/conf/base.js index 7b2ae4f..244c303 100644 --- a/nodejs/conf/base.js +++ b/nodejs/conf/base.js @@ -20,7 +20,16 @@ module.exports = { userBase: 'ou=people,dc=example,dc=com', groupBase: 'ou=groups,dc=example,dc=com', userFilter: '(objectClass=posixAccount)', - userNameAttribute: 'uid' + userNameAttribute: 'uid', + // New users/personal groups (see addPosixAccount/addPosixGroup in + // models/user_ldap.js) get the next uid/gidNumber >= uidGidMin. + // Existing entries >= uidGidReservedFloor are ignored when computing + // that "next available" number, so a deliberately high, easily + // recognizable id (e.g. the bootstrap admin at 10000 — see + // theta-env's bootstrap.js) doesn't drag every real user's id up + // into that same range. + uidGidMin: 1500, + uidGidReservedFloor: 9000, }, oauth: { issuer: '', // falls back to the request host at runtime (routes/index.js) diff --git a/nodejs/models/user_ldap.js b/nodejs/models/user_ldap.js index 6ee7ee3..988b403 100644 --- a/nodejs/models/user_ldap.js +++ b/nodejs/models/user_ldap.js @@ -45,6 +45,23 @@ function escapeLDAPSearchValue(val) { .replace(/\0/g, '\\00'); } +// 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 +// admin deliberately given a high, easily-recognizable id -- see +// theta-env's bootstrap.js) are ignored, so they don't drag every real +// user's id up into that same range. Math.max() on an empty array is +// -Infinity in JS, not 0 -- without the explicit floor here, a fresh +// directory with zero existing entries produces an invalid ("-Infinity") +// LDAP attribute value and the add fails with InvalidSyntaxError. +function nextPosixId(entries, key){ + const existing = entries + .map(i => Number(i[key])) + .filter(n => Number.isFinite(n) && n < conf.uidGidReservedFloor); + + return String(Math.max(conf.uidGidMin - 1, ...existing) + 1); +} + async function addPosixGroup(client, data){ try{ @@ -53,7 +70,7 @@ async function addPosixGroup(client, data){ filter: '(&(objectClass=posixGroup))', })).searchEntries; - data.gidNumber = (Math.max(...groups.map(i => i.gidNumber))+1)+''; + data.gidNumber = nextPosixId(groups, 'gidNumber'); await client.add(`cn=${data.cn},${conf.groupBase}`, { cn: data.cn, @@ -75,7 +92,7 @@ async function addPosixAccount(client, data){ filter: conf.userFilter, })).searchEntries; - data.uidNumber = (Math.max(...people.map(i => i.uidNumber))+1)+''; + data.uidNumber = nextPosixId(people, 'uidNumber'); const entry = { cn: data.cn, @@ -742,7 +759,7 @@ User.login = async function(data){ }; -module.exports = {User, hashPasswordSSHA512}; +module.exports = {User, hashPasswordSSHA512, nextPosixId}; // (async function(){ diff --git a/nodejs/tests/posix_id.test.js b/nodejs/tests/posix_id.test.js new file mode 100644 index 0000000..9c0fe96 --- /dev/null +++ b/nodejs/tests/posix_id.test.js @@ -0,0 +1,39 @@ +'use strict'; + +const { nextPosixId } = require('../models/user_ldap'); + +// Pure logic, no LDAP/Redis needed -- regression coverage for the bug where +// an empty directory (Math.max() on an empty array is -Infinity in JS, not +// 0) produced an invalid "-Infinity" uid/gidNumber and every user creation +// failed with InvalidSyntaxError. Uses the real conf/base.js defaults +// (uidGidMin: 1500, uidGidReservedFloor: 9000). +describe('nextPosixId', () => { + test('starts at uidGidMin (1500) when there are no existing entries', () => { + expect(nextPosixId([], 'uidNumber')).toBe('1500'); + }); + + test('continues from the highest existing value below the reserved floor', () => { + const entries = [{ uidNumber: '1500' }, { uidNumber: '1501' }]; + expect(nextPosixId(entries, 'uidNumber')).toBe('1502'); + }); + + test('ignores entries at/above uidGidReservedFloor (e.g. the bootstrap admin at 10000)', () => { + const entries = [{ uidNumber: '10000' }]; + expect(nextPosixId(entries, 'uidNumber')).toBe('1500'); + }); + + test('a reserved high entry does not affect allocation once real users exist', () => { + const entries = [{ uidNumber: '10000' }, { uidNumber: '1500' }, { uidNumber: '1501' }]; + expect(nextPosixId(entries, 'uidNumber')).toBe('1502'); + }); + + test('ignores non-numeric/missing values instead of producing NaN', () => { + const entries = [{ uidNumber: undefined }, { someOtherField: '1' }]; + expect(nextPosixId(entries, 'uidNumber')).toBe('1500'); + }); + + test('works the same way for gidNumber', () => { + expect(nextPosixId([], 'gidNumber')).toBe('1500'); + expect(nextPosixId([{ gidNumber: '1500' }], 'gidNumber')).toBe('1501'); + }); +});