Fix uid/gidNumber allocation crash, add a configurable id floor (#44)

Reported: creating any user via the API failed with

  {"name":"InvalidSyntaxError","message":"gidNumber: value #0 invalid per syntax Code: 0x15"}

Root cause: addPosixGroup() computes the next gidNumber as
`Math.max(...groups.map(i => i.gidNumber)) + 1`. theta-env's
bootstrap.js creates the first admin via raw ldapadd with a hardcoded
uidNumber/gidNumber (10000) directly on the user entry, but never
creates a matching posixGroup entry -- so on a theta-env-bootstrapped
directory there are zero posixGroup entries, `Math.max()` on an empty
array is `-Infinity` in JS (not 0), and `-Infinity + 1` stringifies to
"-Infinity" -- an invalid LDAP integer, rejected by the directory. This
broke every single user creation, not just this one.

Separately: the reporter's intended scheme is for organically-created
users to start at uidNumber/gidNumber 1500, distinct from the
bootstrap admin's reserved 10000. Fixing the crash with a bare "floor
of 1500" alone wouldn't achieve that, since addPosixAccount's own
Math.max() would still find the admin's posixAccount entry (uidNumber
10000, found via a different, correctly-indexed search) and allocate
10001 for the next user.

Added a shared nextPosixId(entries, key) helper: takes the highest
existing value strictly below conf.ldap.uidGidReservedFloor (default
9000) plus one, or conf.ldap.uidGidMin (default 1500) if there are no
such entries. Ids at/above the reserved floor -- like the bootstrap
admin's 10000 -- are ignored entirely when computing the next
available number, so real users always start at 1500 and grow upward
regardless of the admin's reserved id.

Verified against a real theta-env deployment end to end:
- Reproduced the exact reported crash on a fresh bootstrap
- After the fix: first real user gets uidNumber/gidNumber "1500",
  second gets "1501" -- admin's 10000 never enters the calculation
- New unit tests (nodejs/tests/posix_id.test.js, no LDAP required):
  6/6 pass, covering the empty-array case, the reserved-floor
  exclusion, and the NaN-from-missing-value case
- npm test: 18/18 passing tests still pass (unchanged); the other 155
  failures are pre-existing/environmental (no LDAP server in this
  sandbox) -- confirmed via git stash before starting this fix

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 |

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 |

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)

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(){

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');
+	});
+});