Skip to content

Commit 31d3419

Browse files
authored
chore: Apply strategy pattern to abac (#39769)
1 parent a6327eb commit 31d3419

File tree

8 files changed

+228
-143
lines changed

8 files changed

+228
-143
lines changed

ee/packages/abac/src/can-access-object.spec.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -169,20 +169,13 @@ describe('AbacService.canAccessObject (unit)', () => {
169169
abacLastTimeChecked: within,
170170
});
171171

172-
const internalLogger = (service as any).logger;
173-
const loggerDebug = jest.spyOn(internalLogger, 'debug').mockImplementation(() => undefined);
174172
service.decisionCacheTimeout = ttlSeconds;
175173

176174
const result = await service.canAccessObject(baseRoom as any, baseUser as any, AbacAccessOperation.READ, AbacObjectType.ROOM);
177175

178176
expect(result).toBe(true);
179177
expect(mockUsersFindOne).not.toHaveBeenCalled();
180178
expect(mockSubscriptionsSetAbacLastTimeCheckedByUserIdAndRoomId).not.toHaveBeenCalled();
181-
expect(loggerDebug).toHaveBeenCalledWith({
182-
msg: 'Using cached ABAC decision',
183-
userId: baseUser._id,
184-
roomId: baseRoom._id,
185-
});
186179
});
187180

188181
it('re-evaluates when cache expired (timestamp older than TTL)', async () => {

ee/packages/abac/src/index.ts

Lines changed: 63 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,11 @@ import type {
88
AtLeast,
99
IUser,
1010
ILDAPEntry,
11-
ISubscription,
1211
AbacAuditReason,
1312
} from '@rocket.chat/core-typings';
14-
import { Logger } from '@rocket.chat/logger';
1513
import { Rooms, AbacAttributes, Users, Subscriptions } from '@rocket.chat/models';
1614
import { escapeRegExp } from '@rocket.chat/string-helpers';
17-
import type { Document, FindCursor, UpdateFilter } from 'mongodb';
15+
import type { Document, UpdateFilter } from 'mongodb';
1816
import pLimit from 'p-limit';
1917

2018
import { Audit } from './audit';
@@ -25,34 +23,33 @@ import {
2523
AbacInvalidAttributeValuesError,
2624
AbacUnsupportedObjectTypeError,
2725
AbacUnsupportedOperationError,
28-
OnlyCompliantCanBeAddedToRoomError,
2926
} from './errors';
3027
import {
3128
getAbacRoom,
3229
diffAttributes,
3330
extractAttribute,
3431
diffAttributeSets,
35-
buildCompliantConditions,
36-
buildNonCompliantConditions,
3732
validateAndNormalizeAttributes,
3833
ensureAttributeDefinitionsExist,
39-
buildRoomNonCompliantConditionsFromSubject,
4034
MAX_ABAC_ATTRIBUTE_KEYS,
4135
} from './helper';
36+
import { logger } from './logger';
37+
import type { IPolicyDecisionPoint } from './pdp';
38+
import { LocalPDP, ExternalPDP } from './pdp';
4239

4340
// Limit concurrent user removals to avoid overloading the server with too many operations at once
4441
const limit = pLimit(20);
4542

4643
export class AbacService extends ServiceClass implements IAbacService {
4744
protected name = 'abac';
4845

49-
protected logger: Logger;
46+
private pdp!: IPolicyDecisionPoint;
5047

5148
decisionCacheTimeout = 60; // seconds
5249

5350
constructor() {
5451
super();
55-
this.logger = new Logger('AbacService');
52+
this.setPdpStrategy('local');
5653

5754
this.onSettingChanged('Abac_Cache_Decision_Time_Seconds', async ({ setting }): Promise<void> => {
5855
const { value } = setting;
@@ -63,6 +60,18 @@ export class AbacService extends ServiceClass implements IAbacService {
6360
});
6461
}
6562

63+
setPdpStrategy(strategy: 'local' | 'external'): void {
64+
switch (strategy) {
65+
case 'external':
66+
this.pdp = new ExternalPDP();
67+
break;
68+
case 'local':
69+
default:
70+
this.pdp = new LocalPDP();
71+
break;
72+
}
73+
}
74+
6675
override async started(): Promise<void> {
6776
this.decisionCacheTimeout = await Settings.get<number>('Abac_Cache_Decision_Time_Seconds');
6877
}
@@ -335,7 +344,7 @@ export class AbacService extends ServiceClass implements IAbacService {
335344

336345
const previous: IAbacAttributeDefinition[] = room.abacAttributes || [];
337346
if (diffAttributeSets(previous, normalized).added) {
338-
await this.onRoomAttributesChanged(room, (updated?.abacAttributes as IAbacAttributeDefinition[] | undefined) ?? normalized);
347+
await this.onRoomAttributesChanged(room, updated?.abacAttributes ?? normalized);
339348
}
340349
}
341350

@@ -477,46 +486,6 @@ export class AbacService extends ServiceClass implements IAbacService {
477486
await this.onRoomAttributesChanged(room, updated?.abacAttributes || []);
478487
}
479488

480-
async checkUsernamesMatchAttributes(usernames: string[], attributes: IAbacAttributeDefinition[], object: IRoom): Promise<void> {
481-
if (!usernames.length || !attributes.length) {
482-
return;
483-
}
484-
485-
const nonCompliantUsersFromList = await Users.find(
486-
{
487-
username: { $in: usernames },
488-
$or: buildNonCompliantConditions(attributes),
489-
},
490-
{ projection: { username: 1 } },
491-
)
492-
.map((u) => u.username as string)
493-
.toArray();
494-
495-
const nonCompliantSet = new Set<string>(nonCompliantUsersFromList);
496-
497-
if (nonCompliantSet.size) {
498-
throw new OnlyCompliantCanBeAddedToRoomError();
499-
}
500-
501-
usernames.forEach((username) => {
502-
// TODO: Add room name
503-
void Audit.actionPerformed({ username }, { _id: object._id, name: object.name }, 'system', 'granted-object-access');
504-
});
505-
}
506-
507-
private shouldUseCache(decisionCacheTimeout: number, userSub: ISubscription) {
508-
// Cases:
509-
// 1) Never checked before -> check now
510-
// 2) Checked before, but cache expired -> check now
511-
// 3) Checked before, and cache valid -> use cached decision (subsciprtion exists)
512-
// 4) Cache disabled (0) -> always check
513-
return (
514-
decisionCacheTimeout > 0 &&
515-
userSub.abacLastTimeChecked &&
516-
Date.now() - userSub.abacLastTimeChecked.getTime() < decisionCacheTimeout * 1000
517-
);
518-
}
519-
520489
async canAccessObject(
521490
room: Pick<IRoom, '_id' | 't' | 'teamId' | 'prid' | 'abacAttributes'>,
522491
user: Pick<IUser, '_id'>,
@@ -541,34 +510,42 @@ export class AbacService extends ServiceClass implements IAbacService {
541510
return false;
542511
}
543512

544-
if (this.shouldUseCache(this.decisionCacheTimeout, userSub)) {
545-
this.logger.debug({ msg: 'Using cached ABAC decision', userId: user._id, roomId: room._id });
546-
return !!userSub;
513+
const decision = await this.pdp.canAccessObject(room, user, userSub, this.decisionCacheTimeout);
514+
515+
if (decision.userToRemove) {
516+
// When a user is not compliant, remove them from the room automatically
517+
await this.removeUserFromRoom(room, decision.userToRemove, 'realtime-policy-eval');
547518
}
548519

549-
const isUserCompliant = await Users.findOne(
550-
{
551-
_id: user._id,
552-
$and: buildCompliantConditions(room.abacAttributes),
553-
},
554-
{ projection: { _id: 1 } },
555-
);
520+
return decision.granted;
521+
}
556522

557-
if (!isUserCompliant) {
558-
const fullUser = await Users.findOneById(user._id);
559-
if (!fullUser) {
560-
return false;
561-
}
523+
async checkUsernamesMatchAttributes(usernames: string[], attributes: IAbacAttributeDefinition[], object: IRoom): Promise<void> {
524+
if (!usernames.length || !attributes.length) {
525+
return;
526+
}
562527

563-
// When a user is not compliant, remove them from the room automatically
564-
await this.removeUserFromRoom(room, fullUser, 'realtime-policy-eval');
528+
await this.pdp.checkUsernamesMatchAttributes(usernames, attributes, object);
565529

566-
return false;
567-
}
530+
usernames.forEach((username) => {
531+
void Audit.actionPerformed({ username }, { _id: object._id, name: object.name }, 'system', 'granted-object-access');
532+
});
533+
}
568534

569-
// Set last time the decision was made
570-
await Subscriptions.setAbacLastTimeCheckedByUserIdAndRoomId(user._id, room._id, new Date());
571-
return true;
535+
private async removeUserFromRoom(room: AtLeast<IRoom, '_id'>, user: IUser, reason: AbacAuditReason): Promise<void> {
536+
return Room.removeUserFromRoom(room._id, user, {
537+
skipAppPreEvents: true,
538+
customSystemMessage: 'abac-removed-user-from-room' as const,
539+
})
540+
.then(() => void Audit.actionPerformed({ _id: user._id, username: user.username }, { _id: room._id, name: room.name }, reason))
541+
.catch((err) => {
542+
logger.error({
543+
msg: 'Failed to remove user from ABAC room',
544+
rid: room._id,
545+
err,
546+
reason,
547+
});
548+
});
572549
}
573550

574551
protected async onRoomAttributesChanged(
@@ -578,7 +555,7 @@ export class AbacService extends ServiceClass implements IAbacService {
578555
const rid = room._id;
579556
if (!newAttributes?.length) {
580557
// When a room has no ABAC attributes, it becomes a normal private group and no user removal is necessary
581-
this.logger.warn({
558+
logger.warn({
582559
msg: 'Room ABAC attributes removed. Room is not abac managed anymore',
583560
rid,
584561
});
@@ -587,94 +564,45 @@ export class AbacService extends ServiceClass implements IAbacService {
587564
}
588565

589566
try {
590-
const query = {
591-
__rooms: rid,
592-
$or: buildNonCompliantConditions(newAttributes),
593-
};
594-
595-
const cursor = Users.find(query, { projection: { __rooms: 0 } });
596-
597-
const usersToRemove: string[] = [];
598-
const userRemovalPromises = [];
599-
for await (const doc of cursor) {
600-
usersToRemove.push(doc._id);
601-
userRemovalPromises.push(limit(() => this.removeUserFromRoom(room, doc, 'room-attributes-change')));
602-
}
567+
const nonCompliantUsers = await this.pdp.onRoomAttributesChanged(room, newAttributes);
603568

604-
if (!usersToRemove.length) {
569+
if (!nonCompliantUsers.length) {
605570
return;
606571
}
607572

608-
await Promise.all(userRemovalPromises);
573+
await Promise.all(nonCompliantUsers.map((user) => limit(() => this.removeUserFromRoom(room, user, 'room-attributes-change'))));
609574
} catch (err) {
610-
this.logger.error({
575+
logger.error({
611576
msg: 'Failed to re-evaluate room subscriptions after ABAC attributes changed',
612577
rid,
613578
err,
614579
});
615580
}
616581
}
617582

618-
private async removeUserFromRoom(room: AtLeast<IRoom, '_id'>, user: IUser, reason: AbacAuditReason): Promise<void> {
619-
return Room.removeUserFromRoom(room._id, user, {
620-
skipAppPreEvents: true,
621-
customSystemMessage: 'abac-removed-user-from-room' as const,
622-
})
623-
.then(() => void Audit.actionPerformed({ _id: user._id, username: user.username }, { _id: room._id, name: room.name }, reason))
624-
.catch((err) => {
625-
this.logger.error({
626-
msg: 'Failed to remove user from ABAC room',
627-
rid: room._id,
628-
err,
629-
reason,
630-
});
631-
});
632-
}
633-
634-
private async removeUserFromRoomList(roomList: FindCursor<IRoom>, user: IUser, reason: AbacAuditReason): Promise<void> {
635-
const removalPromises: Promise<void>[] = [];
636-
for await (const room of roomList) {
637-
removalPromises.push(limit(() => this.removeUserFromRoom(room, user, reason)));
638-
}
639-
640-
await Promise.all(removalPromises);
641-
}
642-
643583
protected async onSubjectAttributesChanged(user: IUser, _next: IAbacAttributeDefinition[]): Promise<void> {
644584
if (!user?._id || !Array.isArray(user.__rooms) || !user.__rooms.length) {
645585
return;
646586
}
647-
const roomIds = user.__rooms;
648587

649588
try {
650-
// No attributes: no rooms :(
651-
if (!_next.length) {
652-
const cursor = Rooms.find(
653-
{
654-
_id: { $in: roomIds },
655-
abacAttributes: { $exists: true, $ne: [] },
656-
},
657-
{ projection: { _id: 1 } },
658-
);
659-
660-
return await this.removeUserFromRoomList(cursor, user, 'ldap-sync');
661-
}
589+
const nonCompliantRooms = await this.pdp.onSubjectAttributesChanged(user, _next);
662590

663-
const query = {
664-
_id: { $in: roomIds },
665-
$or: buildRoomNonCompliantConditionsFromSubject(_next),
666-
};
667-
668-
const cursor = Rooms.find(query, { projection: { _id: 1 } });
591+
if (!nonCompliantRooms.length) {
592+
return;
593+
}
669594

670-
return await this.removeUserFromRoomList(cursor, user, 'ldap-sync');
595+
await Promise.all(nonCompliantRooms.map((room) => limit(() => this.removeUserFromRoom(room, user, 'ldap-sync'))));
671596
} catch (err) {
672-
this.logger.error({
597+
logger.error({
673598
msg: 'Failed to query and remove user from non-compliant ABAC rooms',
674599
err,
675600
});
676601
}
677602
}
678603
}
679604

605+
export { LocalPDP, ExternalPDP } from './pdp';
606+
export type { IPolicyDecisionPoint } from './pdp';
607+
680608
export default AbacService;

ee/packages/abac/src/logger.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { Logger } from '@rocket.chat/logger';
2+
3+
export const logger = new Logger('AbacService');
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import type { IAbacAttributeDefinition, IRoom, IUser, AtLeast, ISubscription } from '@rocket.chat/core-typings';
2+
3+
import type { IPolicyDecisionPoint } from './types';
4+
5+
export class ExternalPDP implements IPolicyDecisionPoint {
6+
async canAccessObject(
7+
_room: AtLeast<IRoom, '_id' | 'abacAttributes'>,
8+
_user: AtLeast<IUser, '_id'>,
9+
_userSub: ISubscription,
10+
_decisionCacheTimeout: number,
11+
): Promise<{ granted: boolean; userToRemove?: IUser }> {
12+
throw new Error('ExternalPDP: canAccessObject not implemented');
13+
}
14+
15+
async checkUsernamesMatchAttributes(_usernames: string[], _attributes: IAbacAttributeDefinition[], _object: IRoom): Promise<void> {
16+
throw new Error('ExternalPDP: checkUsernamesMatchAttributes not implemented');
17+
}
18+
19+
async onRoomAttributesChanged(
20+
_room: AtLeast<IRoom, '_id' | 't' | 'teamMain' | 'abacAttributes'>,
21+
_newAttributes: IAbacAttributeDefinition[],
22+
): Promise<IUser[]> {
23+
throw new Error('ExternalPDP: onRoomAttributesChanged not implemented');
24+
}
25+
26+
async onSubjectAttributesChanged(_user: IUser, _next: IAbacAttributeDefinition[]): Promise<IRoom[]> {
27+
throw new Error('ExternalPDP: onSubjectAttributesChanged not implemented');
28+
}
29+
}

0 commit comments

Comments
 (0)