Release 1.11.0: end-user catalog, access requests, nested groups
Closes the end-user half of the directory and adds nested LDAP groups.
The directory could describe the lab but could not tell anyone what they had
or how to reach it, and several of the paths meant to do so were silently
returning nothing:
- GET /api/discovery/me resolved groups from req.user.groups, which does not
exist (req.user carries memberOf), so it returned only isPublic resources
for every human caller -- "My Services" was blank for everyone. The same
read made isDirectoryAdmin() false for real admins.
- The portal's "Discover More Services" called the admin-gated endpoint and
swallowed the 403, so it never rendered for non-admins at all.
- Services reported no address, because /me had reimplemented getMyAccess
without its parent-walking resolution.
Adds the catalog at /, self-service access requests, and admin access
visibility (per-resource counts, and the reverse "what can this user reach").
Nested groups come in two halves. groupOfNames.member already accepts a group
DN, so nesting needs no schema -- what it needs is resolution, which no
released OpenLDAP performs. The all-in-one image therefore builds slapd from a
pinned master commit for the nestgroup overlay, and the app computes the
closure itself when pointed at a server without it. Both paths are covered.
member-values is deliberately left out of nestgroup-flags: it expands `member`
when reading a group, which destroys the distinction between "listed here" and
"reachable through a nested group" and is not recoverable afterwards.
Full suite green in both resolution modes: 215 passed, 2 skipped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,235 @@
|
||||
'use strict';
|
||||
|
||||
// Self-service access requests, end to end: request -> approve -> the grant is
|
||||
// real (visible through /api/discovery/me), plus the guards that keep the flow
|
||||
// from being abused or double-applied.
|
||||
//
|
||||
// The seed `test` user is in app_sso_admin, so it is both the requester and an
|
||||
// eligible approver here. That is unusual in production but exactly what makes
|
||||
// a single-user test able to walk the whole loop.
|
||||
|
||||
const { login, request, app } = require('./setup');
|
||||
|
||||
let token;
|
||||
let siteSlug;
|
||||
let hostSlug;
|
||||
let hostId;
|
||||
let accessGroupCn;
|
||||
|
||||
// Unique per run: these create real LDAP groups and SQL rows, and a rerun must
|
||||
// not collide with the previous run's leftovers.
|
||||
const stamp = Date.now().toString(36);
|
||||
|
||||
beforeAll(async () => {
|
||||
token = await login();
|
||||
|
||||
siteSlug = `artest-site-${stamp}`;
|
||||
const site = await request(app)
|
||||
.post('/api/directory-admin/resources')
|
||||
.set('auth-token', token)
|
||||
.send({ name: `AR Test Site ${stamp}`, slug: siteSlug, kind: 'site' });
|
||||
expect(site.status).toBe(200);
|
||||
|
||||
hostSlug = `artest-host-${stamp}`;
|
||||
const host = await request(app)
|
||||
.post('/api/directory-admin/resources')
|
||||
.set('auth-token', token)
|
||||
.send({
|
||||
name: `AR Test Host ${stamp}`,
|
||||
slug: hostSlug,
|
||||
kind: 'host',
|
||||
parentSlug: siteSlug,
|
||||
metadata: { ip: '10.99.99.9' },
|
||||
});
|
||||
expect(host.status).toBe(200);
|
||||
hostId = host.body.results.id;
|
||||
|
||||
// Creating a host auto-provisions <site>_<slug>_access / _admin.
|
||||
accessGroupCn = `${siteSlug}_${hostSlug}_access`;
|
||||
const adminGroupCn = `${siteSlug}_${hostSlug}_admin`;
|
||||
|
||||
// The creator is seeded into both groups -- groupOfNames requires at least
|
||||
// one member, so Group.add puts the owner's DN there -- and _admin is nested
|
||||
// into _access, so membership of either grants access. A user who already
|
||||
// has access cannot request it (correctly), so step out of both to be a
|
||||
// legitimate requester. Removing only _access would leave the grant intact
|
||||
// through the nesting, which is exactly the kind of thing these tests exist
|
||||
// to catch.
|
||||
for (const cn of [adminGroupCn, accessGroupCn]) {
|
||||
await request(app)
|
||||
.delete(`/api/group/${encodeURIComponent(cn)}/test`)
|
||||
.set('auth-token', token);
|
||||
}
|
||||
});
|
||||
|
||||
describe('Access requests — the request half', () => {
|
||||
let requestId;
|
||||
|
||||
test('POST /api/access-requests creates a pending request on the member group', async () => {
|
||||
const res = await request(app)
|
||||
.post('/api/access-requests')
|
||||
.set('auth-token', token)
|
||||
.send({ slug: hostSlug, note: 'need it for testing' });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.results).toBeDefined();
|
||||
expect(res.body.results.status).toBe('pending');
|
||||
expect(res.body.results.uid).toBe('test');
|
||||
// Must target the _access group, never the _admin one: asking to use a
|
||||
// resource may not silently escalate to administering it.
|
||||
expect(res.body.results.groupCn).toBe(accessGroupCn);
|
||||
requestId = res.body.results.id;
|
||||
});
|
||||
|
||||
test('a second request for the same resource is rejected', async () => {
|
||||
const res = await request(app)
|
||||
.post('/api/access-requests')
|
||||
.set('auth-token', token)
|
||||
.send({ slug: hostSlug });
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
|
||||
test('GET /api/access-requests/mine lists it with the resource attached', async () => {
|
||||
const res = await request(app).get('/api/access-requests/mine').set('auth-token', token);
|
||||
expect(res.status).toBe(200);
|
||||
const found = res.body.results.find(r => r.id === requestId);
|
||||
expect(found).toBeDefined();
|
||||
expect(found.resource.slug).toBe(hostSlug);
|
||||
});
|
||||
|
||||
test('GET /api/access-requests shows it to an approver', async () => {
|
||||
const res = await request(app).get('/api/access-requests').set('auth-token', token);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.results.some(r => r.id === requestId)).toBe(true);
|
||||
});
|
||||
|
||||
test('requesting an unknown resource is a 404', async () => {
|
||||
const res = await request(app)
|
||||
.post('/api/access-requests')
|
||||
.set('auth-token', token)
|
||||
.send({ slug: `no-such-resource-${stamp}` });
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Access requests — approval actually grants', () => {
|
||||
let requestId;
|
||||
|
||||
beforeAll(async () => {
|
||||
const mine = await request(app).get('/api/access-requests/mine').set('auth-token', token);
|
||||
const pending = mine.body.results.find(r => r.groupCn === accessGroupCn && r.status === 'pending');
|
||||
requestId = pending && pending.id;
|
||||
expect(requestId).toBeDefined();
|
||||
});
|
||||
|
||||
test('the resource is NOT in /api/discovery/me before approval', async () => {
|
||||
const res = await request(app).get('/api/discovery/me').set('auth-token', token);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.results.some(r => r.id === hostId)).toBe(false);
|
||||
});
|
||||
|
||||
test('POST /:id/approve marks it approved', async () => {
|
||||
const res = await request(app)
|
||||
.post(`/api/access-requests/${requestId}/approve`)
|
||||
.set('auth-token', token)
|
||||
.send({ decisionNote: 'ok' });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.results.status).toBe('approved');
|
||||
expect(res.body.results.decidedBy).toBe('test');
|
||||
});
|
||||
|
||||
test('approving twice is rejected', async () => {
|
||||
const res = await request(app)
|
||||
.post(`/api/access-requests/${requestId}/approve`)
|
||||
.set('auth-token', token)
|
||||
.send({});
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
|
||||
// The payoff, and the regression guard for the user.groups bug: /me resolved
|
||||
// groups off req.user.groups, which does not exist on a User (it carries
|
||||
// memberOf), so this endpoint used to return only isPublic resources no
|
||||
// matter what the caller was actually a member of.
|
||||
test('the resource IS in /api/discovery/me after approval', async () => {
|
||||
const res = await request(app).get('/api/discovery/me').set('auth-token', token);
|
||||
expect(res.status).toBe(200);
|
||||
const found = res.body.results.find(r => r.id === hostId);
|
||||
expect(found).toBeDefined();
|
||||
// And it answers "how do I reach it" rather than just naming the thing.
|
||||
expect(found.resolvedAddress).toBe('10.99.99.9');
|
||||
});
|
||||
|
||||
test('an already-granted resource cannot be requested again', async () => {
|
||||
const res = await request(app)
|
||||
.post('/api/access-requests')
|
||||
.set('auth-token', token)
|
||||
.send({ slug: hostSlug });
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Admin access visibility', () => {
|
||||
test('GET /api/directory-admin/access-summary counts the host\'s groups + members', async () => {
|
||||
const res = await request(app)
|
||||
.get('/api/directory-admin/access-summary')
|
||||
.set('auth-token', token);
|
||||
expect(res.status).toBe(200);
|
||||
const summary = res.body.results[hostId];
|
||||
expect(summary).toBeDefined();
|
||||
// _access and _admin were both auto-created and linked.
|
||||
expect(summary.groups.length).toBe(2);
|
||||
expect(summary.groups.every(g => g.exists)).toBe(true);
|
||||
// The approval above put `test` in the access group.
|
||||
expect(summary.memberCount).toBeGreaterThanOrEqual(1);
|
||||
});
|
||||
|
||||
test('GET /api/directory-admin/user-access/:uid answers the reverse question', async () => {
|
||||
const res = await request(app)
|
||||
.get('/api/directory-admin/user-access/test')
|
||||
.set('auth-token', token);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.results.uid).toBe('test');
|
||||
const entry = res.body.results.resources.find(r => r.id === hostId);
|
||||
expect(entry).toBeDefined();
|
||||
expect(entry.groupCn).toBe(accessGroupCn);
|
||||
});
|
||||
|
||||
test('user-access for an unknown uid is a 404', async () => {
|
||||
const res = await request(app)
|
||||
.get('/api/directory-admin/user-access/definitely-not-a-user')
|
||||
.set('auth-token', token);
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Access requests — withdrawal', () => {
|
||||
test('a requester can withdraw their own pending request', async () => {
|
||||
// A second resource, so this does not disturb the approved one above.
|
||||
const slug = `artest-host2-${stamp}`;
|
||||
const host = await request(app)
|
||||
.post('/api/directory-admin/resources')
|
||||
.set('auth-token', token)
|
||||
.send({ name: `AR Test Host2 ${stamp}`, slug, kind: 'host', parentSlug: siteSlug });
|
||||
expect(host.status).toBe(200);
|
||||
|
||||
// Same as the top-level setup: step out of the auto-created groups the
|
||||
// creator is seeded into, or this is a request for access already held.
|
||||
for (const cn of [`${siteSlug}_${slug}_admin`, `${siteSlug}_${slug}_access`]) {
|
||||
await request(app)
|
||||
.delete(`/api/group/${encodeURIComponent(cn)}/test`)
|
||||
.set('auth-token', token);
|
||||
}
|
||||
|
||||
const created = await request(app)
|
||||
.post('/api/access-requests')
|
||||
.set('auth-token', token)
|
||||
.send({ slug });
|
||||
expect(created.status).toBe(200);
|
||||
|
||||
const res = await request(app)
|
||||
.delete(`/api/access-requests/${created.body.results.id}`)
|
||||
.set('auth-token', token);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.results.status).toBe('cancelled');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,136 @@
|
||||
'use strict';
|
||||
|
||||
// Nested groups: the API for putting a group inside a group, the cycle guard,
|
||||
// and the thing that makes it worth doing -- membership resolving transitively
|
||||
// through the chain.
|
||||
//
|
||||
// Fixture note that is easy to get wrong: groupOfNames requires at least one
|
||||
// member, so whoever creates a group is seeded into it. `test` creates all
|
||||
// three groups here and would therefore be a *direct* member of each, which
|
||||
// would make "resolved via nesting" indistinguishable from "was already in it".
|
||||
// Setup below strips that back so test's only direct membership is the
|
||||
// innermost group -- and the strip has to happen after nesting, or removing the
|
||||
// sole member would violate the objectClass.
|
||||
|
||||
const { login, request, app } = require('./setup');
|
||||
|
||||
let token;
|
||||
const stamp = Date.now().toString(36);
|
||||
const A = `nesttest-a-${stamp}`; // outermost
|
||||
const B = `nesttest-b-${stamp}`; // middle
|
||||
const C = `nesttest-c-${stamp}`; // innermost, holds the user
|
||||
// A second group nested into A purely so that un-nesting B later does not
|
||||
// empty A -- groupOfNames requires at least one member, and the API correctly
|
||||
// refuses (409) rather than leaving an invalid entry behind.
|
||||
const D = `nesttest-d-${stamp}`;
|
||||
|
||||
async function nest(parent, child) {
|
||||
return request(app).put(`/api/group/${parent}/nested/${child}`).set('auth-token', token).send({});
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
token = await login();
|
||||
|
||||
for (const cn of [A, B, C, D]) {
|
||||
const res = await request(app)
|
||||
.post('/api/group')
|
||||
.set('auth-token', token)
|
||||
.send({ name: cn, description: `nesting test ${cn}` });
|
||||
expect([200, 201]).toContain(res.status);
|
||||
}
|
||||
|
||||
expect((await nest(A, B)).status).toBe(200);
|
||||
expect((await nest(B, C)).status).toBe(200);
|
||||
expect((await nest(A, D)).status).toBe(200);
|
||||
|
||||
// Now that A holds B and B holds C, neither would be left memberless.
|
||||
for (const cn of [A, B]) {
|
||||
const res = await request(app).delete(`/api/group/${cn}/test`).set('auth-token', token);
|
||||
expect(res.status).toBe(200);
|
||||
}
|
||||
});
|
||||
|
||||
describe('Nested groups — API guards', () => {
|
||||
test('nesting the same pair twice is a 409, not a duplicate', async () => {
|
||||
const res = await nest(A, B);
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
|
||||
test('a group cannot contain itself', async () => {
|
||||
const res = await nest(A, A);
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
// The guard that matters: without it the resolver would silently return a
|
||||
// depth-capped answer instead of an error anyone would notice.
|
||||
test('a direct cycle is refused (A contains B, so B may not contain A)', async () => {
|
||||
const res = await nest(B, A);
|
||||
expect(res.status).toBe(409);
|
||||
expect(res.body.message).toMatch(/loop/i);
|
||||
});
|
||||
|
||||
test('an indirect cycle is refused too (A>B>C, so C may not contain A)', async () => {
|
||||
const res = await nest(C, A);
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Nested groups — resolution', () => {
|
||||
test('membership resolves through the whole chain', async () => {
|
||||
const res = await request(app).get('/api/group?member=test').set('auth-token', token);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.results).toContain(C); // direct
|
||||
expect(res.body.results).toContain(B); // via C
|
||||
expect(res.body.results).toContain(A); // via B -> C
|
||||
});
|
||||
|
||||
test('GET /:group/effective separates direct members from nested ones', async () => {
|
||||
const res = await request(app).get(`/api/group/${A}/effective`).set('auth-token', token);
|
||||
expect(res.status).toBe(200);
|
||||
const { direct, nestedGroups, effective } = res.body.results;
|
||||
|
||||
expect(nestedGroups.map(g => g.cn)).toContain(B);
|
||||
// `direct` is users only -- a nested group must never be reported as one.
|
||||
expect(direct.every(dn => !/,ou=groups,/i.test(dn))).toBe(true);
|
||||
// test is not listed on A at all, yet is effectively a member two levels down.
|
||||
expect(direct.some(dn => /cn=test,/i.test(dn))).toBe(false);
|
||||
expect(effective.some(dn => /cn=test,/i.test(dn))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Nested groups — un-nesting', () => {
|
||||
test('DELETE removes the nesting and the membership it carried', async () => {
|
||||
// Before: A holds B (which holds C, which holds test) and D.
|
||||
const before = await request(app).get(`/api/group/${A}/effective`).set('auth-token', token);
|
||||
expect(before.body.results.nestedGroups.map(g => g.cn)).toContain(B);
|
||||
expect(before.body.results.effective.some(dn => /cn=test,/i.test(dn))).toBe(true);
|
||||
|
||||
const res = await request(app)
|
||||
.delete(`/api/group/${A}/nested/${B}`)
|
||||
.set('auth-token', token);
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
const after = await request(app).get(`/api/group/${A}/effective`).set('auth-token', token);
|
||||
expect(after.body.results.nestedGroups.map(g => g.cn)).not.toContain(B);
|
||||
expect(after.body.results.nestedGroups.map(g => g.cn)).toContain(D); // untouched
|
||||
|
||||
// test still resolves to B and C directly/through C; only the A path via
|
||||
// B is gone. It is deliberately NOT asserted that test loses A entirely:
|
||||
// D is also nested in A and test created D, so that path remains -- which
|
||||
// is itself a fair illustration of why "who can reach this" has to be
|
||||
// computed rather than eyeballed.
|
||||
const groups = await request(app).get('/api/group?member=test').set('auth-token', token);
|
||||
expect(groups.body.results).toContain(C);
|
||||
expect(groups.body.results).toContain(B);
|
||||
});
|
||||
|
||||
test('un-nesting the last member is refused rather than emptying the group', async () => {
|
||||
// B now holds only C. Removing it would leave B with no members at all,
|
||||
// which groupOfNames forbids.
|
||||
const res = await request(app)
|
||||
.delete(`/api/group/${B}/nested/${C}`)
|
||||
.set('auth-token', token);
|
||||
expect(res.status).toBe(409);
|
||||
expect(res.body.message).toMatch(/at least one member/i);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user