0c2f38f0fe
routes/group.js's add/removeMember never called User.clearCache(), unlike the isServiceAccount handling in routes/user.js (which does this deliberately, with a comment explaining exactly why). isServiceAccount is derived at User.get() time from app_sso_service_account membership and cached for 5 minutes -- so adding or removing a user from ANY group via this route left group-derived state (isServiceAccount, and by extension anything else that reads memberOf off a cached User) stale for up to 5 minutes. In production this manifested as a real user's account appearing to "vanish": users.ejs's People tab filters out anything with isServiceAccount truthy, so once that user's membership in app_sso_service_account changed, they'd disappear from the tab anyone actually looks at for up to 5 minutes -- looking exactly like data loss, though the account was never touched. Found by investigating a live "lost users" report: the account had isServiceAccount: 'yes' and was in fact still fully present, just hidden. This does not explain how the account came to be a member of app_sso_service_account in the first place (unresolved -- possibly a manual/accidental group-membership change via the Groups UI, which has no guardrail against adding a real person to what's meant to be a marker group for non-person accounts). It does fix a real correctness gap: any admin group-membership change now takes effect immediately instead of on a timer. Verified against a real LDAP+Redis harness: the new test fails on the unfixed code (stale isServiceAccount immediately after the PUT) and passes with the fix. Full suite: 189/191 passing (2 pre-existing skips). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
231 lines
7.2 KiB
JavaScript
231 lines
7.2 KiB
JavaScript
'use strict';
|
|
|
|
const { TEST_CREDS, login, request, app } = require('./setup');
|
|
|
|
const TEST_GROUP = {
|
|
name: 'test_jest_group',
|
|
description: 'Created by automated test suite',
|
|
};
|
|
|
|
let token;
|
|
let firstGroupCN;
|
|
|
|
beforeAll(async () => {
|
|
token = await login();
|
|
// Clean up any leftover group from a previous failed run
|
|
await request(app).delete(`/api/group/${TEST_GROUP.name}`).set('auth-token', token);
|
|
});
|
|
|
|
afterAll(async () => {
|
|
await request(app).delete(`/api/group/${TEST_GROUP.name}`).set('auth-token', token);
|
|
});
|
|
|
|
describe('Groups — GET /api/group/', () => {
|
|
test('requires auth — 401 without token', async () => {
|
|
const res = await request(app).get('/api/group/');
|
|
expect(res.status).toBe(401);
|
|
});
|
|
|
|
test('returns group list for authenticated user', async () => {
|
|
const res = await request(app)
|
|
.get('/api/group/')
|
|
.set('auth-token', token);
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(Array.isArray(res.body.results)).toBe(true);
|
|
expect(res.body.results.length).toBeGreaterThan(0);
|
|
|
|
firstGroupCN = res.body.results[0];
|
|
});
|
|
|
|
test('detail=true returns full group objects', async () => {
|
|
const res = await request(app)
|
|
.get('/api/group/?detail=true')
|
|
.set('auth-token', token);
|
|
|
|
expect(res.status).toBe(200);
|
|
const group = res.body.results[0];
|
|
expect(group).toHaveProperty('cn');
|
|
expect(group).toHaveProperty('dn');
|
|
expect(group).toHaveProperty('description');
|
|
});
|
|
});
|
|
|
|
describe('Groups — GET /api/group/:cn', () => {
|
|
test('returns a single group by cn', async () => {
|
|
const res = await request(app)
|
|
.get(`/api/group/${firstGroupCN}`)
|
|
.set('auth-token', token);
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.results).toHaveProperty('cn', firstGroupCN);
|
|
});
|
|
|
|
test('unknown cn returns an error', async () => {
|
|
const res = await request(app)
|
|
.get('/api/group/no_such_group_xyz')
|
|
.set('auth-token', token);
|
|
|
|
expect(res.status).toBeGreaterThanOrEqual(400);
|
|
});
|
|
});
|
|
|
|
describe('Groups — POST /api/group/ (create)', () => {
|
|
test('creates a new group', async () => {
|
|
const res = await request(app)
|
|
.post('/api/group/')
|
|
.set('auth-token', token)
|
|
.send(TEST_GROUP);
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body).toHaveProperty('message');
|
|
expect(res.body.message).toMatch(TEST_GROUP.name);
|
|
});
|
|
|
|
test('new group appears in list', async () => {
|
|
const res = await request(app)
|
|
.get('/api/group/')
|
|
.set('auth-token', token);
|
|
|
|
expect(res.body.results).toContain(TEST_GROUP.name);
|
|
});
|
|
|
|
test('requires admin — 401 without token', async () => {
|
|
const res = await request(app)
|
|
.post('/api/group/')
|
|
.send(TEST_GROUP);
|
|
|
|
expect(res.status).toBe(401);
|
|
});
|
|
});
|
|
|
|
// groupOfNames requires at least one member. The creator (test) is auto-added.
|
|
// We test add/remove with a second known LDAP user to avoid the last-member constraint.
|
|
const MEMBER_UID = 'wmantly';
|
|
|
|
describe('Groups — member management', () => {
|
|
test('creator is already a member after group creation', async () => {
|
|
const res = await request(app)
|
|
.get(`/api/group/${TEST_GROUP.name}`)
|
|
.set('auth-token', token);
|
|
|
|
const group = res.body.results;
|
|
const members = Array.isArray(group.member) ? group.member : [group.member];
|
|
expect(members.some(dn => dn && dn.includes(TEST_CREDS.uid))).toBe(true);
|
|
});
|
|
|
|
test('PUT /:group/:uid — add second member', async () => {
|
|
const res = await request(app)
|
|
.put(`/api/group/${TEST_GROUP.name}/${MEMBER_UID}`)
|
|
.set('auth-token', token);
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body).toHaveProperty('message');
|
|
});
|
|
|
|
test('second member appears in group detail', async () => {
|
|
const res = await request(app)
|
|
.get(`/api/group/${TEST_GROUP.name}`)
|
|
.set('auth-token', token);
|
|
|
|
const group = res.body.results;
|
|
const members = Array.isArray(group.member) ? group.member : [group.member];
|
|
expect(members.some(dn => dn && dn.includes(MEMBER_UID))).toBe(true);
|
|
});
|
|
|
|
test('DELETE /:group/:uid — remove second member', async () => {
|
|
const res = await request(app)
|
|
.delete(`/api/group/${TEST_GROUP.name}/${MEMBER_UID}`)
|
|
.set('auth-token', token);
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body).toHaveProperty('message');
|
|
});
|
|
|
|
test('second member no longer in group after removal', async () => {
|
|
const res = await request(app)
|
|
.get(`/api/group/${TEST_GROUP.name}`)
|
|
.set('auth-token', token);
|
|
|
|
const group = res.body.results;
|
|
const members = Array.isArray(group.member) ? group.member : [group.member];
|
|
expect(members.some(dn => dn && dn.includes(MEMBER_UID))).toBe(false);
|
|
});
|
|
|
|
// Regression: adding/removing a member here didn't clear User's LRU
|
|
// cache (ttl 5 minutes), so isServiceAccount -- derived from
|
|
// app_sso_service_account membership at GET /api/user/:uid time -- could
|
|
// stay wrong for up to 5 minutes after the group change. In production
|
|
// this hid a real person's account from the Users page's "People" tab
|
|
// (it filters out anything with isServiceAccount) for however long the
|
|
// stale cache entry lived, which looked exactly like the account had
|
|
// vanished.
|
|
test('PUT app_sso_service_account/:uid immediately flips isServiceAccount (no stale cache)', async () => {
|
|
const added = await request(app)
|
|
.put(`/api/group/app_sso_service_account/${MEMBER_UID}`)
|
|
.set('auth-token', token);
|
|
expect(added.status).toBe(200);
|
|
|
|
const afterAdd = await request(app).get(`/api/user/${MEMBER_UID}`).set('auth-token', token);
|
|
expect(afterAdd.body.results.isServiceAccount).toBeTruthy();
|
|
|
|
const removed = await request(app)
|
|
.delete(`/api/group/app_sso_service_account/${MEMBER_UID}`)
|
|
.set('auth-token', token);
|
|
expect(removed.status).toBe(200);
|
|
|
|
const afterRemove = await request(app).get(`/api/user/${MEMBER_UID}`).set('auth-token', token);
|
|
expect(afterRemove.body.results.isServiceAccount).toBeFalsy();
|
|
});
|
|
});
|
|
|
|
describe('Groups — owner management', () => {
|
|
test('PUT /owner/:group/:uid — add second owner', async () => {
|
|
const res = await request(app)
|
|
.put(`/api/group/owner/${TEST_GROUP.name}/${MEMBER_UID}`)
|
|
.set('auth-token', token);
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body).toHaveProperty('message');
|
|
});
|
|
|
|
test('second owner appears in group detail', async () => {
|
|
const res = await request(app)
|
|
.get(`/api/group/${TEST_GROUP.name}`)
|
|
.set('auth-token', token);
|
|
|
|
const group = res.body.results;
|
|
const owners = Array.isArray(group.owner) ? group.owner : [group.owner];
|
|
expect(owners.some(dn => dn && dn.includes(MEMBER_UID))).toBe(true);
|
|
});
|
|
|
|
test('DELETE /owner/:group/:uid — remove second owner', async () => {
|
|
const res = await request(app)
|
|
.delete(`/api/group/owner/${TEST_GROUP.name}/${MEMBER_UID}`)
|
|
.set('auth-token', token);
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body).toHaveProperty('message');
|
|
});
|
|
});
|
|
|
|
describe('Groups — DELETE /api/group/:cn', () => {
|
|
test('deletes the test group', async () => {
|
|
const res = await request(app)
|
|
.delete(`/api/group/${TEST_GROUP.name}`)
|
|
.set('auth-token', token);
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body).toHaveProperty('message');
|
|
expect(res.body.message).toMatch(TEST_GROUP.name);
|
|
});
|
|
|
|
test('group no longer appears in list', async () => {
|
|
const res = await request(app)
|
|
.get('/api/group/')
|
|
.set('auth-token', token);
|
|
|
|
expect(res.body.results).not.toContain(TEST_GROUP.name);
|
|
});
|
|
});
|