Skip to content

Commit 94e3da0

Browse files
KevLehmanMartinSchoeler
authored andcommitted
fix: Add name to object being audited on access grant (#37856)
1 parent bd28882 commit 94e3da0

File tree

4 files changed

+20
-10
lines changed

4 files changed

+20
-10
lines changed

apps/meteor/ee/server/hooks/abac/beforeAddUserToRoom.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,5 @@ beforeAddUserToRoom.patch(async (prev, users, room, actor) => {
1818
throw new Error('error-room-is-abac-managed');
1919
}
2020

21-
await Abac.checkUsernamesMatchAttributes(validUsers as string[], room.abacAttributes, room._id);
21+
await Abac.checkUsernamesMatchAttributes(validUsers as string[], room.abacAttributes, room);
2222
});

ee/packages/abac/src/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ export class AbacService extends ServiceClass implements IAbacService {
463463
await this.onRoomAttributesChanged(room, updated?.abacAttributes || []);
464464
}
465465

466-
async checkUsernamesMatchAttributes(usernames: string[], attributes: IAbacAttributeDefinition[], objectId: string): Promise<void> {
466+
async checkUsernamesMatchAttributes(usernames: string[], attributes: IAbacAttributeDefinition[], object: IRoom): Promise<void> {
467467
if (!usernames.length || !attributes.length) {
468468
return;
469469
}
@@ -486,7 +486,7 @@ export class AbacService extends ServiceClass implements IAbacService {
486486

487487
usernames.forEach((username) => {
488488
// TODO: Add room name
489-
void Audit.actionPerformed({ username }, { _id: objectId }, 'system', 'granted-object-access');
489+
void Audit.actionPerformed({ username }, { _id: object._id, name: object.name }, 'system', 'granted-object-access');
490490
});
491491
}
492492

ee/packages/abac/src/service.spec.ts

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,12 +1037,14 @@ describe('AbacService (unit)', () => {
10371037
const attributes = [{ key: 'dept', values: ['eng'] }];
10381038

10391039
it('returns early (no query) when usernames array is empty', async () => {
1040-
await expect(service.checkUsernamesMatchAttributes([], attributes as any, 'objectId')).resolves.toBeUndefined();
1040+
await expect(
1041+
service.checkUsernamesMatchAttributes([], attributes as any, { _id: 'xxxxx', name: 'name' } as any),
1042+
).resolves.toBeUndefined();
10411043
expect(mockUsersFind).not.toHaveBeenCalled();
10421044
});
10431045

10441046
it('returns early (no query) when attributes array is empty', async () => {
1045-
await expect(service.checkUsernamesMatchAttributes(['alice'], [], 'objectId')).resolves.toBeUndefined();
1047+
await expect(service.checkUsernamesMatchAttributes(['alice'], [], { _id: 'xxxxx', name: 'name' } as any)).resolves.toBeUndefined();
10461048
expect(mockUsersFind).not.toHaveBeenCalled();
10471049
});
10481050

@@ -1054,7 +1056,9 @@ describe('AbacService (unit)', () => {
10541056
}),
10551057
}));
10561058

1057-
await expect(service.checkUsernamesMatchAttributes(usernames, attributes as any, 'objectId')).resolves.toBeUndefined();
1059+
await expect(
1060+
service.checkUsernamesMatchAttributes(usernames, attributes as any, { _id: 'xxxxx', name: 'name' } as any),
1061+
).resolves.toBeUndefined();
10581062

10591063
expect(mockUsersFind).toHaveBeenCalledWith(
10601064
{
@@ -1085,7 +1089,9 @@ describe('AbacService (unit)', () => {
10851089
}),
10861090
}));
10871091

1088-
await expect(service.checkUsernamesMatchAttributes(usernames, attributes as any, 'objectId')).rejects.toMatchObject({
1092+
await expect(
1093+
service.checkUsernamesMatchAttributes(usernames, attributes as any, { _id: 'xxxxx', name: 'name' } as any),
1094+
).rejects.toMatchObject({
10891095
code: 'error-only-compliant-users-can-be-added-to-abac-rooms',
10901096
});
10911097
});
@@ -1099,7 +1105,9 @@ describe('AbacService (unit)', () => {
10991105
}),
11001106
}));
11011107

1102-
await expect(service.checkUsernamesMatchAttributes(usernames, attributes as any, 'objectId')).resolves.toBeUndefined();
1108+
await expect(
1109+
service.checkUsernamesMatchAttributes(usernames, attributes as any, { _id: 'xxxxx', name: 'name' } as any),
1110+
).resolves.toBeUndefined();
11031111

11041112
expect(mockCreateAuditServerEvent).toHaveBeenCalledTimes(usernames.length);
11051113
const calledUsernames = mockCreateAuditServerEvent.mock.calls.map(([, payload]: any[]) => payload?.subject?.username).filter(Boolean);
@@ -1116,7 +1124,9 @@ describe('AbacService (unit)', () => {
11161124
}),
11171125
}));
11181126

1119-
await expect(service.checkUsernamesMatchAttributes(usernames, attributes as any, 'objectId')).rejects.toMatchObject({
1127+
await expect(
1128+
service.checkUsernamesMatchAttributes(usernames, attributes as any, { _id: 'xxxxx', name: 'name' } as any),
1129+
).rejects.toMatchObject({
11201130
code: 'error-only-compliant-users-can-be-added-to-abac-rooms',
11211131
});
11221132

packages/core-services/src/types/IAbacService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export interface IAbacService {
3838
removeRoomAbacAttribute(rid: string, key: string, actor: AbacActor | undefined): Promise<void>;
3939
addRoomAbacAttributeByKey(rid: string, key: string, values: string[], actor: AbacActor | undefined): Promise<void>;
4040
replaceRoomAbacAttributeByKey(rid: string, key: string, values: string[], actor: AbacActor | undefined): Promise<void>;
41-
checkUsernamesMatchAttributes(usernames: string[], attributes: IAbacAttributeDefinition[], objectId: string): Promise<void>;
41+
checkUsernamesMatchAttributes(usernames: string[], attributes: IAbacAttributeDefinition[], object: IRoom): Promise<void>;
4242
canAccessObject(
4343
room: Pick<IRoom, '_id' | 't' | 'teamId' | 'prid' | 'abacAttributes'>,
4444
user: Pick<IUser, '_id'>,

0 commit comments

Comments
 (0)