fix: use verbatim resource slugs in group names (matches access-request tests + live convention)

The group naming inserts a kind segment (resourceGroupCns(site, kind, slug, level)),
but the access-request tests + the live directory convention are verbatim
({site}_{slug}_{level} -- the kind is carried in the resource slug, e.g. host_theta-env).
For bare test slugs this produced site_x_host_artest-host_x_access instead of the
expected site_x_artest-host_x_access, so the requester was never removed from the
auto-provisioned access group and every request 409'd. resourceGroupCns is now
(site, slug, level) with the verbatim slug; the kind is used only to pick the
aggregate the group nests into.

CHANGELOG.md

@@ -1,5 +1,5 @@
 # v1.26.0
-- feat: complete the group model (docs/GROUPS.md) — `god_admin` is now seeded into LDAP and nested into `app_super_admin`; every site auto-provisions `{site}_super_admin`, `{site}_hosts_*`/`{site}_apps_*` aggregates and `{site}_everyone`; per-resource `_admin`/`_access` groups are nested into the site aggregates so the inheritance lattice exists in LDAP, not just in the resolver; service resources map to the group model's `app` kind (`site_local_app_<slug>_*` instead of the kind-less `site_local_<slug>_*`). Site/aggregate groups are self-healed idempotently on every Directory load, so a directory seeded by an older release picks them up without a rebuild.
+- feat: complete the group model (docs/GROUPS.md) — `god_admin` is now seeded into LDAP and nested into `app_super_admin`; every site auto-provisions `{site}_super_admin`, `{site}_hosts_*`/`{site}_apps_*` aggregates and `{site}_everyone`; per-resource `_admin`/`_access` groups (named `{site}_{slug}_{level}`, the kind carried in the resource slug) are nested into the site aggregates so the inheritance lattice exists in LDAP, not just in the resolver. Site/aggregate groups are self-healed idempotently on every Directory load, so a directory seeded by an older release picks them up without a rebuild.
 - feat: the naming convention is now enforced server-side — `POST /api/directory-admin/groups` rejects a group CN that isn't a valid group for the target resource (its own `_admin`/`_access`/capability, a site aggregate, a site-level group, or `god_admin`), so the free-text field can no longer mint `*_accessmember`-style names
 - feat: `god_admin` is managed from the Directory — the site resource modal surfaces `god_admin` + the site-level groups as associated groups, so its members (and the site's) are editable right there
 - fix: Directory agent status dots no longer paint every host red when the `/api/agent/nodes` endpoint is unreachable (older app or transient outage) — they now show a neutral grey "agent service unreachable" instead of a false alarm

nodejs/routes/api_directory_admin.js

@@ -47,15 +47,6 @@ function groupKind(resource) {
   return null;
 }
 
-// The group-model slug for a resource: strip the kind prefix that directory
-// resource slugs carry (`site_local` -> `local`, `host_theta-env` -> `theta-env`)
-// so the group-model builders can re-apply their own `{site}_{kind}_{slug}`
-// structure without double-prefixing. Services are stored without a prefix, so
-// this is a no-op for them.
-function resourceSlug(slug) {
-  return String(slug || '').replace(/^(site|host|app)_/, '');
-}
-
 // Create a groupOfNames if it doesn't already exist. Idempotent; `ownerDn`
 // seeds the mandatory first member. Returns true when created.
 async function ensureGroup(name, ownerDn, description) {
@@ -122,16 +113,18 @@ async function ensureSiteGroups(siteSlug, ownerDn, siteName, siteResourceId) {
 }
 
 // Provision the per-resource groups for a host/app and nest them into the site
-// aggregates (so a site/aggregate admin reaches this resource by membership):
+// aggregates (so a site/aggregate admin reaches this resource by membership).
+// The specific group name uses the resource's slug verbatim
+// (`{site}_{slug}_{level}` -- the kind is carried in the slug, e.g. `host_theta-env`);
+// `kind` (host/app) selects which aggregate the group nests into:
 //
-//   {site}_{kind}_{slug}_admin  -> {site}_{kind}_{slug}_access
+//   {site}_{slug}_admin  -> {site}_{slug}_access
-//   {site}_{kind}_{slug}_admin  -> {site}_{kind}s_admin    (aggregate)
+//   {site}_{slug}_admin  -> {site}_{kind}s_admin    (aggregate)
-//   {site}_{kind}_{slug}_access -> {site}_{kind}s_access   (aggregate)
+//   {site}_{slug}_access -> {site}_{kind}s_access   (aggregate)
-//   app_super_admin             -> {site}_{kind}_{slug}_admin  (legacy cross-app)
+//   app_super_admin      -> {site}_{slug}_admin     (legacy cross-app)
 async function provisionResourceGroups(resource, kind, siteSlug, ownerDn) {
-  const slug = resourceSlug(resource.slug);
+  const accessCn = groups.resourceGroupCns(siteSlug, resource.slug, 'access');
-  const accessCn = groups.resourceGroupCns(siteSlug, kind, slug, 'access');
+  const adminCn = groups.resourceGroupCns(siteSlug, resource.slug, 'admin');
-  const adminCn = groups.resourceGroupCns(siteSlug, kind, slug, 'admin');
 
   await ensureGroup(accessCn, ownerDn, `Access group for ${resource.name}`);
   await ensureGroup(adminCn, ownerDn, `Admin group for ${resource.name}`);
@@ -165,14 +158,14 @@ function validGroupCnsForResource(resource, siteSlug) {
   }
   const kind = groupKind(resource); // 'host'|'app'|null
   if (kind) {
-    const slug = resourceSlug(resource.slug);
+    const slug = resource.slug; // verbatim (kind is carried in the slug)
-    valid.add(groups.resourceGroupCns(siteSlug, kind, slug, 'admin'));
+    valid.add(groups.resourceGroupCns(siteSlug, slug, 'admin'));
-    valid.add(groups.resourceGroupCns(siteSlug, kind, slug, 'access'));
+    valid.add(groups.resourceGroupCns(siteSlug, slug, 'access'));
     valid.add(groups.aggregateGroupCns(siteSlug, kind, 'admin'));
     valid.add(groups.aggregateGroupCns(siteSlug, kind, 'access'));
     valid.add(groups.siteSuperAdminCns(siteSlug));
     valid.add(groups.siteEveryoneCns(siteSlug));
-    return { valid, capRe: new RegExp(`^${siteSlug}_(${kind}_${slug}_|${kind}s_)[a-z0-9-]+$`) };
+    return { valid, capRe: new RegExp(`^${siteSlug}_(${slug}_|${kind}s_)[a-z0-9-]+$`) };
   }
   // oauth/container etc. — only the global god_admin makes sense to pin here.
   valid.add(groups.siteSuperAdminCns(siteSlug));

nodejs/tests/groups.test.js

@@ -12,13 +12,12 @@ const {
 	GOD_ADMIN,
 } = require('../utils/groups');
 
-// Resource fixtures use already-normalized slugs (the group-model builders treat
+// Resource fixtures mirror the directory's real slugs: hosts carry a `host_`
-// the site + resource slugs as opaque, not re-slugified -- see groups.js). Raw
+// prefix, services/apps are stored bare. The group-model builders use these
-// display names like "Main Office" are normalized by the resource model before
+// verbatim (no re-slugifying, no kind insertion) -- see groups.js.
-// they reach the resolver.
+const HOST = { site: 'main-office', kind: 'host', slug: 'host_web-01' };
-const HOST = { site: 'main-office', kind: 'host', slug: 'web-01' };
 const APP = { site: 'main-office', kind: 'app', slug: 'emby' };
-const OTHER_SITE_HOST = { site: 'branch-office', kind: 'host', slug: 'db' };
+const OTHER_SITE_HOST = { site: 'branch-office', kind: 'host', slug: 'host_db' };
 
 describe('slugify', () => {
 	test('lowercases, spaces and underscores become hyphens, no leading/trailing dash', () => {
@@ -28,18 +27,14 @@ describe('slugify', () => {
 		expect(slugify('  Mixed CASE--name  ')).toBe('mixed-case-name');
 		expect(slugify('')).toBe('');
 	});
-	test('never contains an underscore (the structural delimiter)', () => {
-		expect(slugify('a_b_c')).not.toContain('_');
-		expect(resourceGroupCns('Main Office', 'host', 'Web 01', 'access')).not.toContain('__');
-	});
 });
 
 describe('group cn builders', () => {
-	test('per-resource uses singular kind', () => {
+	test('per-resource uses the resource slug verbatim (kind is carried in the slug)', () => {
-		expect(resourceGroupCns('main-office', 'host', 'web-01', 'admin')).toBe('main-office_host_web-01_admin');
+		expect(resourceGroupCns('main-office', 'host_web-01', 'admin')).toBe('main-office_host_web-01_admin');
-		expect(resourceGroupCns('main-office', 'app', 'emby', 'access')).toBe('main-office_app_emby_access');
+		expect(resourceGroupCns('main-office', 'emby', 'access')).toBe('main-office_emby_access');
 	});
-	test('aggregate uses plural kind', () => {
+	test('aggregate uses the plural kind', () => {
 		expect(aggregateGroupCns('main-office', 'host', 'admin')).toBe('main-office_hosts_admin');
 		expect(aggregateGroupCns('main-office', 'app', 'access')).toBe('main-office_apps_access');
 	});
@@ -53,10 +48,10 @@ describe('group cn builders', () => {
 		expect(siteSuperAdminCns('site_local')).toBe('site_local_super_admin');
 		expect(siteEveryoneCns('site_local')).toBe('site_local_everyone');
 		expect(aggregateGroupCns('site_local', 'host', 'admin')).toBe('site_local_hosts_admin');
-		expect(resourceGroupCns('site_local', 'host', 'theta-env', 'access')).toBe('site_local_host_theta-env_access');
+		expect(resourceGroupCns('site_local', 'host_theta-env', 'access')).toBe('site_local_host_theta-env_access');
 	});
-	test('invalid kind throws', () => {
+	test('invalid kind throws (aggregates only — per-resource has no kind arg)', () => {
-		expect(() => resourceGroupCns('s', 'service', 'x', 'admin')).toThrow();
+		expect(() => aggregateGroupCns('s', 'service', 'admin')).toThrow();
 	});
 });
 
@@ -94,32 +89,32 @@ describe('hasPermission — inheritance', () => {
 	});
 
 	test('specific host group grants only that host', () => {
-		const cn = resourceGroupCns('main-office', 'host', 'web-01', 'admin');
+		const cn = resourceGroupCns('main-office', 'host_web-01', 'admin');
 		expect(hasPermission([cn], HOST, 'admin')).toBe(true);
 		expect(hasPermission([cn], OTHER_SITE_HOST, 'admin')).toBe(false);
 	});
 
 	test('admin implies access; access does not imply admin', () => {
-		expect(hasPermission([resourceGroupCns('main-office', 'host', 'web-01', 'admin')], HOST, 'access')).toBe(true);
+		expect(hasPermission([resourceGroupCns('main-office', 'host_web-01', 'admin')], HOST, 'access')).toBe(true);
-		expect(hasPermission([resourceGroupCns('main-office', 'host', 'web-01', 'access')], HOST, 'admin')).toBe(false);
+		expect(hasPermission([resourceGroupCns('main-office', 'host_web-01', 'access')], HOST, 'admin')).toBe(false);
 	});
 
 	test('capabilities are exact — admin does not grant a capability', () => {
-		expect(hasPermission([resourceGroupCns('main-office', 'host', 'web-01', 'reboot')], HOST, 'reboot')).toBe(true);
+		expect(hasPermission([resourceGroupCns('main-office', 'host_web-01', 'reboot')], HOST, 'reboot')).toBe(true);
-		expect(hasPermission([resourceGroupCns('main-office', 'host', 'web-01', 'admin')], HOST, 'reboot')).toBe(false);
+		expect(hasPermission([resourceGroupCns('main-office', 'host_web-01', 'admin')], HOST, 'reboot')).toBe(false);
 		// aggregate capability
 		expect(hasPermission(['main-office_hosts_reboot'], HOST, 'reboot')).toBe(true);
 	});
 
 	test('hosts and apps are orthogonal namespaces', () => {
-		const hostAdmin = resourceGroupCns('main-office', 'host', 'web-01', 'admin');
+		const hostAdmin = resourceGroupCns('main-office', 'host_web-01', 'admin');
 		expect(hasPermission([hostAdmin], APP, 'access')).toBe(false);
-		const appAdmin = resourceGroupCns('main-office', 'app', 'emby', 'admin');
+		const appAdmin = resourceGroupCns('main-office', 'emby', 'admin');
 		expect(hasPermission([appAdmin], APP, 'access')).toBe(true);
 	});
 
 	test('cross-site isolation', () => {
-		const mainHostAdmin = resourceGroupCns('main-office', 'host', 'web-01', 'admin');
+		const mainHostAdmin = resourceGroupCns('main-office', 'host_web-01', 'admin');
 		expect(hasPermission([mainHostAdmin], OTHER_SITE_HOST, 'access')).toBe(false);
 		expect(hasPermission(['branch-office_hosts_admin'], OTHER_SITE_HOST, 'admin')).toBe(true);
 	});

nodejs/utils/groups.js

@@ -41,16 +41,17 @@ function assertKind(kind) {
   if (!KINDS.includes(kind)) throw new Error(`invalid resource kind: ${kind} (must be host or app)`);
 }
 
-// {site}_host_<slug>_<level> / {site}_app_<slug>_<level>
+// {site}_{slug}_{level} — the per-resource group for one resource.
 //
-// `site` is the site resource's slug verbatim (e.g. `site_local`, or a clean
+// Both `site` and `slug` are the resource slugs verbatim (e.g. `site_local`,
-// `main-office`), NOT slugified: directory resource slugs may legitimately carry
+// `host_theta-env`), NOT slugified or kind-inserted: directory resource slugs
-// a kind prefix (`site_local`, `host_theta-env`), and re-slugifying those would
+// carry their kind as a prefix (`host_theta-env`), so `site_local` + `host_theta-env`
-// corrupt the delimiter (site_local -> site-local). Callers pass already-
+// yields `site_local_host_theta-env_access`. Services are stored without a
-// normalized site + kind-stripped resource slugs.
+// prefix (`sso-manager`), yielding `site_local_sso-manager_access`. This is the
-function resourceGroupCns(site, kind, slug, level) {
+// convention the auto-provisioner, the resolver, and the access-request tests
-  assertKind(kind);
+// all share -- re-slugifying or inserting a kind would double the delimiter.
-  return `${site}_${kind}_${slugify(slug)}_${level}`;
+function resourceGroupCns(site, slug, level) {
+  return `${site}_${slug}_${level}`;
 }
 
 // {site}_hosts_<level> / {site}_apps_<level> (plural kind — the aggregate).
@@ -93,12 +94,11 @@ function levelGrants(level, wanted) {
 // granted groups (see permission.onResource). This keeps the function pure over
 // the user's membership only.
 function hasPermission(memberOf, resource, level) {
-  // `site` is used verbatim (resource slugs may carry a kind prefix, e.g.
+  // `site` and `slug` are used verbatim (resource slugs may carry a kind prefix,
-  // `site_local`) -- see resourceGroupCns; the resource `slug` is expected
+  // e.g. `site_local` / `host_theta-env`) -- see resourceGroupCns.
-  // kind-stripped and already-normalized, so slugify() is a harmless guard.
   const site = resource && resource.site;
   const kind = resource && resource.kind;
-  const slug = slugify(resource && resource.slug);
+  const slug = resource && resource.slug;
   const set = new Set(memberOf || []);
 
   if (set.has(GOD_ADMIN)) return true;
@@ -107,13 +107,13 @@ function hasPermission(memberOf, resource, level) {
   if (isKnownLevel(level)) {
     // admin / access
     if (set.has(aggregateGroupCns(site, kind, level))) return true;
-    if (set.has(resourceGroupCns(site, kind, slug, level))) return true;
+    if (set.has(resourceGroupCns(site, slug, level))) return true;
     if (level === 'access' && hasPermission(memberOf, resource, 'admin')) return true;
     return false;
   }
   // Opaque capability — exact aggregate or specific grant only.
   if (set.has(aggregateGroupCns(site, kind, level))) return true;
-  if (set.has(resourceGroupCns(site, kind, slug, level))) return true;
+  if (set.has(resourceGroupCns(site, slug, level))) return true;
   return false;
 }