Skip to content

Commit 30729ab

Browse files
KevLehmanKevLehman
committed
more fixes
1 parent 9e1dfa2 commit 30729ab

File tree

2 files changed

+276
-209
lines changed

2 files changed

+276
-209
lines changed

‎ee/packages/abac/src/subject-attributes-validations.spec.ts‎

Lines changed: 94 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@ jest.mock('@rocket.chat/core-services', () => ({
1616

1717
const makeUser = (overrides: Partial<IUser> = {}): IUser =>
1818
({
19-
_id: `user-${Math.random().toString(36).substring(2, 15)}`,
20-
username: `user${Math.random().toString(36).substring(2, 15)}`,
19+
_id: `user-fixed-id-${Math.random()}`,
20+
username: 'user-fixed-username',
2121
roles: [],
2222
type: 'user',
2323
active: true,
24-
createdAt: new Date(),
25-
_updatedAt: new Date(),
24+
createdAt: new Date(0),
25+
_updatedAt: new Date(0),
2626
...overrides,
2727
}) as IUser;
2828

@@ -35,6 +35,21 @@ describe('Subject Attributes validation', () => {
3535
let db: Db;
3636
let mongo: MongoMemoryServer;
3737
let client: MongoClient;
38+
const service = new AbacService();
39+
40+
let roomsCol: Collection<any>;
41+
let usersCol: Collection<any>;
42+
43+
const insertRooms = async (rooms: { _id: string; abacAttributes?: IAbacAttributeDefinition[] }[]) => {
44+
await roomsCol.insertMany(
45+
rooms.map((room) => ({
46+
_id: room._id,
47+
name: room._id,
48+
t: 'p',
49+
abacAttributes: room.abacAttributes,
50+
})),
51+
);
52+
};
3853

3954
beforeAll(async () => {
4055
mongo = await MongoMemoryServer.create();
@@ -48,17 +63,27 @@ describe('Subject Attributes validation', () => {
4863
registerModel('IServerEventsModel', () => new ServerEventsRaw(db));
4964

5065
// @ts-expect-error - ignore
51-
await db.collection('abac_dummy_init').insertOne({ _id: 'init', createdAt: new Date() });
66+
await db.collection('abac_dummy_init').insertOne({ _id: 'init', createdAt: new Date(0) });
67+
68+
roomsCol = db.collection('rocketchat_room');
69+
usersCol = db.collection('users');
5270
}, 30_000);
5371

5472
afterAll(async () => {
5573
await client.close();
5674
await mongo.stop();
5775
});
5876

59-
describe('AbacService.addSubjectAttributes (unit)', () => {
60-
const service = new AbacService();
77+
beforeEach(async () => {
78+
// Clear state between tests while reusing the same in-memory DB & models
79+
await Promise.all([usersCol.deleteMany({}), roomsCol.deleteMany({})]);
80+
});
81+
82+
const insertUser = async (user: IUser) => {
83+
await usersCol.insertOne(user);
84+
};
6185

86+
describe('AbacService.addSubjectAttributes (unit)', () => {
6287
describe('early returns and no-ops', () => {
6388
it('returns early when user has no _id', async () => {
6489
const user = makeUser({ _id: undefined });
@@ -70,7 +95,7 @@ describe('Subject Attributes validation', () => {
7095

7196
it('does nothing (no update) when map produces no attributes and user had none', async () => {
7297
const user = makeUser({ abacAttributes: undefined });
73-
await Users.insertOne(user);
98+
await insertUser(user);
7499
const ldap = makeLdap({ group: '' });
75100
await service.addSubjectAttributes(user, ldap, { group: 'dept' });
76101
const updated = await Users.findOneById(user._id, { projection: { abacAttributes: 1 } });
@@ -82,7 +107,7 @@ describe('Subject Attributes validation', () => {
82107
describe('building and setting attributes', () => {
83108
it('merges multiple LDAP keys mapping to the same ABAC key, deduplicating values', async () => {
84109
const user = makeUser();
85-
await Users.insertOne(user);
110+
await insertUser(user);
86111
const ldap = makeLdap({
87112
memberOf: ['eng', 'sales', 'eng'],
88113
department: ['sales', 'support'],
@@ -95,7 +120,7 @@ describe('Subject Attributes validation', () => {
95120

96121
it('creates distinct ABAC attributes for different mapped keys preserving insertion order', async () => {
97122
const user = makeUser();
98-
await Users.insertOne(user);
123+
await insertUser(user);
99124
const ldap = makeLdap({
100125
groups: ['alpha', 'beta'],
101126
regionCodes: ['emea', 'apac'],
@@ -113,7 +138,7 @@ describe('Subject Attributes validation', () => {
113138

114139
it('merges array and string LDAP values into one attribute', async () => {
115140
const user = makeUser();
116-
await Users.insertOne(user);
141+
await insertUser(user);
117142
const ldap = makeLdap({ deptCode: 'eng', deptName: ['engineering', 'eng'] });
118143
const map = { deptCode: 'dept', deptName: 'dept' };
119144
await service.addSubjectAttributes(user, ldap, map);
@@ -125,7 +150,7 @@ describe('Subject Attributes validation', () => {
125150
describe('unsetting attributes when none extracted', () => {
126151
it('unsets abacAttributes when user previously had attributes but now extracts none', async () => {
127152
const user = makeUser({ abacAttributes: [{ key: 'dept', values: ['eng', 'sales'] }] });
128-
await Users.insertOne(user);
153+
await insertUser(user);
129154
const ldap = makeLdap({ other: ['x'] });
130155
const map = { missing: 'dept' };
131156
await service.addSubjectAttributes(user, ldap, map);
@@ -135,7 +160,7 @@ describe('Subject Attributes validation', () => {
135160

136161
it('does not unset when user had no prior attributes and extraction yields none', async () => {
137162
const user = makeUser({ abacAttributes: [] });
138-
await Users.insertOne(user);
163+
await insertUser(user);
139164
const ldap = makeLdap({});
140165
const map = { missing: 'dept' };
141166
await service.addSubjectAttributes(user, ldap, map);
@@ -147,7 +172,7 @@ describe('Subject Attributes validation', () => {
147172
describe('loss detection triggering hook (attribute changes)', () => {
148173
it('updates attributes reducing values on loss', async () => {
149174
const user = makeUser({ abacAttributes: [{ key: 'dept', values: ['eng', 'qa'] }] });
150-
await Users.insertOne(user);
175+
await insertUser(user);
151176
const ldap = makeLdap({ memberOf: ['eng'] });
152177
await service.addSubjectAttributes(user, ldap, { memberOf: 'dept' });
153178
const updated = await Users.findOneById(user._id, { projection: { abacAttributes: 1 } });
@@ -161,7 +186,7 @@ describe('Subject Attributes validation', () => {
161186
{ key: 'region', values: ['emea'] },
162187
],
163188
});
164-
await Users.insertOne(user);
189+
await insertUser(user);
165190
const ldap = makeLdap({ department: ['eng'] });
166191
await service.addSubjectAttributes(user, ldap, { department: 'dept' });
167192
const updated = await Users.findOneById(user._id, { projection: { abacAttributes: 1 } });
@@ -170,7 +195,7 @@ describe('Subject Attributes validation', () => {
170195

171196
it('gains new values without triggering loss logic', async () => {
172197
const user = makeUser({ abacAttributes: [{ key: 'dept', values: ['eng'] }] });
173-
await Users.insertOne(user);
198+
await insertUser(user);
174199
const ldap = makeLdap({ memberOf: ['eng', 'qa'] });
175200
await service.addSubjectAttributes(user, ldap, { memberOf: 'dept' });
176201
const updated = await Users.findOneById(user._id, { projection: { abacAttributes: 1 } });
@@ -179,7 +204,7 @@ describe('Subject Attributes validation', () => {
179204

180205
it('keeps attributes unchanged when only ordering differs', async () => {
181206
const user = makeUser({ abacAttributes: [{ key: 'dept', values: ['eng', 'qa'] }] });
182-
await Users.insertOne(user);
207+
await insertUser(user);
183208
const ldap = makeLdap({ memberOf: ['qa', 'eng'] });
184209
await service.addSubjectAttributes(user, ldap, { memberOf: 'dept' });
185210
const updated = await Users.findOneById(user._id, { projection: { abacAttributes: 1 } });
@@ -189,7 +214,7 @@ describe('Subject Attributes validation', () => {
189214

190215
it('merges duplicate LDAP mapping keys retaining union of values', async () => {
191216
const user = makeUser({ abacAttributes: [{ key: 'dept', values: ['eng', 'sales'] }] });
192-
await Users.insertOne(user);
217+
await insertUser(user);
193218
const ldap = makeLdap({ deptA: ['eng', 'sales'], deptB: ['eng'] });
194219
await service.addSubjectAttributes(user, ldap, { deptA: 'dept', deptB: 'dept' });
195220
const updated = await Users.findOneById(user._id, { projection: { abacAttributes: 1 } });
@@ -198,53 +223,42 @@ describe('Subject Attributes validation', () => {
198223
});
199224

200225
describe('input immutability', () => {
226+
let sharedUser: IUser;
227+
let original: IAbacAttributeDefinition[];
228+
let clone: IAbacAttributeDefinition[];
229+
230+
beforeAll(async () => {
231+
original = [{ key: 'dept', values: ['eng', 'sales'] }] as IAbacAttributeDefinition[];
232+
clone = JSON.parse(JSON.stringify(original));
233+
sharedUser = makeUser({ abacAttributes: original });
234+
await insertUser(sharedUser);
235+
});
236+
237+
afterAll(async () => {
238+
await usersCol.deleteOne({ _id: sharedUser._id });
239+
});
240+
201241
it('does not mutate original user.abacAttributes array reference contents', async () => {
202-
const original = [{ key: 'dept', values: ['eng', 'sales'] }] as IAbacAttributeDefinition[];
203-
const user = makeUser({ abacAttributes: original });
204-
await Users.insertOne(user);
205-
const clone = JSON.parse(JSON.stringify(original));
206242
const ldap = makeLdap({ memberOf: ['eng', 'sales', 'support'] });
207-
await service.addSubjectAttributes(user, ldap, { memberOf: 'dept' });
243+
await service.addSubjectAttributes(sharedUser, ldap, { memberOf: 'dept' });
208244
expect(original).toEqual(clone);
209245
});
210246
});
211247
});
212248

213249
describe('AbacService.addSubjectAttributes (room removals)', () => {
214-
let service: AbacService;
215-
let roomsCol: Collection<any>;
216-
let usersCol: Collection<any>;
217-
218250
const originalCoreServices = jest.requireMock('@rocket.chat/core-services');
219251
originalCoreServices.Room.removeUserFromRoom = async (rid: string, user: IUser) => {
220252
// @ts-expect-error - test
221253
await usersCol.updateOne({ _id: user._id }, { $pull: { __rooms: rid } });
222254
};
223255

224-
const insertRoom = async (room: { _id: string; abacAttributes?: IAbacAttributeDefinition[] }) =>
225-
roomsCol.insertOne({
226-
_id: room._id,
227-
name: room._id,
228-
t: 'p',
229-
abacAttributes: room.abacAttributes,
230-
});
231-
232-
const insertUser = async (user: IUser & { __rooms?: string[] }) =>
256+
const insertUserForRemovalTests = async (user: IUser & { __rooms?: string[] }) =>
233257
usersCol.insertOne({
234258
...user,
235259
__rooms: user.__rooms || [],
236260
});
237261

238-
beforeAll(async () => {
239-
roomsCol = db.collection('rocketchat_room');
240-
usersCol = db.collection('users');
241-
await Promise.all([roomsCol.deleteMany({}), usersCol.deleteMany({})]);
242-
});
243-
244-
beforeEach(() => {
245-
service = new AbacService();
246-
});
247-
248262
it('removes user from rooms whose attributes become non-compliant after losing a value', async () => {
249263
const user: IUser = {
250264
_id: 'u-loss',
@@ -261,12 +275,12 @@ describe('Subject Attributes validation', () => {
261275
// Rooms:
262276
// rKeep requires only 'eng' (will remain compliant)
263277
// rRemove requires both 'eng' and 'qa' (will become non-compliant after loss)
264-
await Promise.all([
265-
insertRoom({ _id: 'rKeep', abacAttributes: [{ key: 'dept', values: ['eng'] }] }),
266-
insertRoom({ _id: 'rRemove', abacAttributes: [{ key: 'dept', values: ['eng', 'qa'] }] }),
278+
await insertRooms([
279+
{ _id: 'rKeep', abacAttributes: [{ key: 'dept', values: ['eng'] }] },
280+
{ _id: 'rRemove', abacAttributes: [{ key: 'dept', values: ['eng', 'qa'] }] },
267281
]);
268282

269-
await insertUser({ ...user, __rooms: ['rKeep', 'rRemove'] });
283+
await insertUserForRemovalTests({ ...user, __rooms: ['rKeep', 'rRemove'] });
270284

271285
const ldap: ILDAPEntry = {
272286
memberOf: ['eng'],
@@ -301,19 +315,19 @@ describe('Subject Attributes validation', () => {
301315
// rDeptOnly -> only dept (will stay)
302316
// rRegionOnly -> region only (will be removed after region key loss)
303317
// rBoth -> both dept & region (will be removed)
304-
await Promise.all([
305-
insertRoom({ _id: 'rDeptOnly', abacAttributes: [{ key: 'dept', values: ['eng'] }] }),
306-
insertRoom({ _id: 'rRegionOnly', abacAttributes: [{ key: 'region', values: ['emea'] }] }),
307-
insertRoom({
318+
await insertRooms([
319+
{ _id: 'rDeptOnly', abacAttributes: [{ key: 'dept', values: ['eng'] }] },
320+
{ _id: 'rRegionOnly', abacAttributes: [{ key: 'region', values: ['emea'] }] },
321+
{
308322
_id: 'rBoth',
309323
abacAttributes: [
310324
{ key: 'dept', values: ['eng'] },
311325
{ key: 'region', values: ['emea'] },
312326
],
313-
}),
327+
},
314328
]);
315329

316-
await insertUser({ ...user, __rooms: ['rDeptOnly', 'rRegionOnly', 'rBoth'] });
330+
await insertUserForRemovalTests({ ...user, __rooms: ['rDeptOnly', 'rRegionOnly', 'rBoth'] });
317331

318332
const ldap: ILDAPEntry = {
319333
department: ['eng'],
@@ -341,12 +355,12 @@ describe('Subject Attributes validation', () => {
341355
__rooms: ['rGrowthA', 'rGrowthB'],
342356
};
343357

344-
await Promise.all([
345-
insertRoom({ _id: 'rGrowthA', abacAttributes: [{ key: 'dept', values: ['eng'] }] }),
346-
insertRoom({ _id: 'rGrowthB', abacAttributes: [{ key: 'dept', values: ['eng', 'qa'] }] }), // superset; still compliant after growth
358+
await insertRooms([
359+
{ _id: 'rGrowthA', abacAttributes: [{ key: 'dept', values: ['eng'] }] },
360+
{ _id: 'rGrowthB', abacAttributes: [{ key: 'dept', values: ['eng', 'qa'] }] }, // superset; still compliant after growth
347361
]);
348362

349-
await insertUser({ ...user, __rooms: ['rGrowthA', 'rGrowthB'] });
363+
await insertUserForRemovalTests({ ...user, __rooms: ['rGrowthA', 'rGrowthB'] });
350364

351365
const ldap: ILDAPEntry = {
352366
memberOf: ['eng', 'qa'],
@@ -376,21 +390,21 @@ describe('Subject Attributes validation', () => {
376390
__rooms: ['rExtraKeyRoom', 'rBaseline'],
377391
};
378392

379-
await Promise.all([
380-
insertRoom({
393+
await insertRooms([
394+
{
381395
_id: 'rExtraKeyRoom',
382396
abacAttributes: [
383397
{ key: 'dept', values: ['eng', 'sales'] },
384398
{ key: 'project', values: ['X'] },
385399
],
386-
}),
387-
insertRoom({
400+
},
401+
{
388402
_id: 'rBaseline',
389403
abacAttributes: [{ key: 'dept', values: ['eng', 'sales'] }],
390-
}),
404+
},
391405
]);
392406

393-
await insertUser({ ...user, __rooms: ['rExtraKeyRoom', 'rBaseline'] });
407+
await insertUserForRemovalTests({ ...user, __rooms: ['rExtraKeyRoom', 'rBaseline'] });
394408

395409
const ldap: ILDAPEntry = {
396410
deptCodes: ['eng', 'sales'],
@@ -417,12 +431,12 @@ describe('Subject Attributes validation', () => {
417431
__rooms: ['rAny1', 'rAny2'],
418432
};
419433

420-
await Promise.all([
421-
insertRoom({ _id: 'rAny1', abacAttributes: [{ key: 'dept', values: ['eng'] }] }),
422-
insertRoom({ _id: 'rAny2', abacAttributes: [{ key: 'dept', values: ['eng', 'qa'] }] }),
434+
await insertRooms([
435+
{ _id: 'rAny1', abacAttributes: [{ key: 'dept', values: ['eng'] }] },
436+
{ _id: 'rAny2', abacAttributes: [{ key: 'dept', values: ['eng', 'qa'] }] },
423437
]);
424438

425-
await insertUser({ ...user, __rooms: ['rAny1', 'rAny2'] });
439+
await insertUserForRemovalTests({ ...user, __rooms: ['rAny1', 'rAny2'] });
426440

427441
const ldap: ILDAPEntry = {
428442
unrelated: ['x'],
@@ -454,15 +468,17 @@ describe('Subject Attributes validation', () => {
454468
__rooms: ['rDeptRegion'],
455469
};
456470

457-
await insertRoom({
458-
_id: 'rDeptRegion',
459-
abacAttributes: [
460-
{ key: 'dept', values: ['eng'] },
461-
{ key: 'region', values: ['emea'] },
462-
],
463-
});
471+
await insertRooms([
472+
{
473+
_id: 'rDeptRegion',
474+
abacAttributes: [
475+
{ key: 'dept', values: ['eng'] },
476+
{ key: 'region', values: ['emea'] },
477+
],
478+
},
479+
]);
464480

465-
await insertUser({ ...user, __rooms: ['rDeptRegion'] });
481+
await insertUserForRemovalTests({ ...user, __rooms: ['rDeptRegion'] });
466482

467483
const ldap: ILDAPEntry = {
468484
department: ['eng', 'ceo'],

0 commit comments

Comments
 (0)