Skip to content

Commit 5a8945d

Browse files
KevLehmanKevLehman
authored
chore: Delegate user sync to PDP (#40836)
1 parent 24dc6ec commit 5a8945d

12 files changed

Lines changed: 315 additions & 16 deletions

File tree

‎apps/meteor/ee/server/api/abac/index.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { AbacAttributeStoreExternalError, getPdpHealthErrorCode } from '@rocket.chat/abac';
2-
import { Abac, LDAPEnterprise } from '@rocket.chat/core-services';
2+
import { Abac } from '@rocket.chat/core-services';
33
import type { AbacActor } from '@rocket.chat/core-services';
44
import type { IServerEvents, IUser } from '@rocket.chat/core-typings';
5-
import { ServerEvents, Users } from '@rocket.chat/models';
5+
import { ServerEvents } from '@rocket.chat/models';
66
import { validateUnauthorizedErrorResponse } from '@rocket.chat/rest-typings/src/v1/Ajv';
77
import { convertSubObjectsIntoPaths } from '@rocket.chat/tools';
88

@@ -209,7 +209,7 @@ const abacEndpoints = API.v1
209209
{
210210
authRequired: true,
211211
permissionsRequired: ['abac-management', 'manage-abac-admin-room-attributes'],
212-
license: ['abac', 'ldap-enterprise'],
212+
license: ['abac'],
213213
body: POSTAbacUsersSyncBodySchema,
214214
response: {
215215
200: GenericSuccessSchema,
@@ -225,7 +225,7 @@ const abacEndpoints = API.v1
225225

226226
const { usernames, ids, emails, ldapIds } = this.bodyParams;
227227

228-
await LDAPEnterprise.syncUsersAbacAttributes(Users.findUsersByIdentifiers({ usernames, ids, emails, ldapIds }));
228+
await Abac.reevaluateUsers({ usernames, ids, emails, ldapIds });
229229

230230
return API.v1.success();
231231
},

‎apps/meteor/ee/server/local-services/ldap/service.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { ServiceClassInternal, type ILDAPEEService } from '@rocket.chat/core-services';
22
import type { IUser } from '@rocket.chat/core-typings';
3+
import { Users } from '@rocket.chat/models';
34
import type { FindCursor } from 'mongodb';
45

56
import { LDAPEEManager } from '../../lib/ldap/Manager';
@@ -30,4 +31,8 @@ export class LDAPEEService extends ServiceClassInternal implements ILDAPEEServic
3031
async syncUsersAbacAttributes(users: FindCursor<IUser>): Promise<void> {
3132
return LDAPEEManager.syncUsersAbacAttributes(users);
3233
}
34+
35+
async syncUsersAbacAttributesByIds(userIds: string[]): Promise<void> {
36+
return LDAPEEManager.syncUsersAbacAttributes(Users.findUsersByIdentifiers({ ids: userIds }));
37+
}
3338
}

‎apps/meteor/tests/end-to-end/api/abac.ts‎

Lines changed: 154 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { expect } from 'chai';
44
import { before, after, describe, it } from 'mocha';
55
import { MongoClient } from 'mongodb';
66

7-
import { getCredentials, request, credentials, methodCall } from '../../data/api-data';
7+
import { api, getCredentials, request, credentials, methodCall } from '../../data/api-data';
88
import { sleep } from '../../data/livechat/utils';
99
import {
1010
mockServerHealthy,
@@ -190,7 +190,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
190190

191191
it('POST /abac/users/sync should return 403', async () => {
192192
await request
193-
.post(`${v1}/abac/users/sync`)
193+
.post(api('abac/users/sync'))
194194
.set(credentials)
195195
.send({ usernames: ['x'] })
196196
.expect(403);
@@ -1451,6 +1451,17 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
14511451
});
14521452
});
14531453

1454+
it('POST /abac/users/sync should fail with error-abac-not-enabled', async () => {
1455+
await request
1456+
.post(api('abac/users/sync'))
1457+
.set(credentials)
1458+
.send({ ids: ['no-such-user-id'] })
1459+
.expect(400)
1460+
.expect((res) => {
1461+
expect(res.body.error).to.include('error-abac-not-enabled');
1462+
});
1463+
});
1464+
14541465
after(async () => {
14551466
await updateSetting('ABAC_Enabled', true);
14561467
});
@@ -1832,6 +1843,27 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
18321843
});
18331844
});
18341845

1846+
describe('POST /abac/users/sync (strategy-agnostic)', () => {
1847+
before(async () => {
1848+
await updateSetting('ABAC_Enabled', true);
1849+
});
1850+
1851+
after(async () => {
1852+
await updateSetting('ABAC_Enabled', false);
1853+
});
1854+
1855+
it('responds 200 with success:true when ABAC_Enabled=true and PDP type=local (no-match id)', async () => {
1856+
await request
1857+
.post(api('abac/users/sync'))
1858+
.set(credentials)
1859+
.send({ ids: ['no-such-user-id'] })
1860+
.expect(200)
1861+
.expect((res) => {
1862+
expect(res.body).to.have.property('success', true);
1863+
});
1864+
});
1865+
});
1866+
18351867
describe('Room access (invite, addition)', () => {
18361868
let roomWithoutAttr: IRoom;
18371869
let roomWithAttr: IRoom;
@@ -2503,7 +2535,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
25032535

25042536
it('should sync ABAC attributes for SOME users via /abac/users/sync', async () => {
25052537
await request
2506-
.post(`${v1}/abac/users/sync`)
2538+
.post(api('abac/users/sync'))
25072539
.set(credentials)
25082540
.send({
25092541
usernames: ['david.scott', 'gene.cernan'],
@@ -2533,7 +2565,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
25332565
it('should fail /abac/users/sync when more than 100 usernames are provided', async () => {
25342566
const usernames = Array.from({ length: 101 }, (_, i) => `user_${i}@example.com`);
25352567
await request
2536-
.post(`${v1}/abac/users/sync`)
2568+
.post(api('abac/users/sync'))
25372569
.set(credentials)
25382570
.send({
25392571
usernames,
@@ -2547,7 +2579,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
25472579
it('should fail /abac/users/sync when more than 100 ids are provided', async () => {
25482580
const ids = Array.from({ length: 101 }, (_, i) => `id_${i}`);
25492581
await request
2550-
.post(`${v1}/abac/users/sync`)
2582+
.post(api('abac/users/sync'))
25512583
.set(credentials)
25522584
.send({
25532585
ids,
@@ -2561,7 +2593,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
25612593
it('should fail /abac/users/sync when more than 100 emails are provided', async () => {
25622594
const emails = Array.from({ length: 101 }, (_, i) => `user_${i}@example.com`);
25632595
await request
2564-
.post(`${v1}/abac/users/sync`)
2596+
.post(api('abac/users/sync'))
25652597
.set(credentials)
25662598
.send({
25672599
emails,
@@ -2575,7 +2607,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
25752607
it('should fail /abac/users/sync when more than 100 ldapIds are provided', async () => {
25762608
const ldapIds = Array.from({ length: 101 }, (_, i) => `ldap_${i}`);
25772609
await request
2578-
.post(`${v1}/abac/users/sync`)
2610+
.post(api('abac/users/sync'))
25792611
.set(credentials)
25802612
.send({
25812613
ldapIds,
@@ -2589,7 +2621,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
25892621
it('should succeed /abac/users/sync when exactly 100 usernames are provided (boundary)', async () => {
25902622
const usernames = Array.from({ length: 100 }, (_, i) => `boundary_user_${i}`);
25912623
await request
2592-
.post(`${v1}/abac/users/sync`)
2624+
.post(api('abac/users/sync'))
25932625
.set(credentials)
25942626
.send({
25952627
usernames,
@@ -2667,7 +2699,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
26672699
expect(sergeiInitialAttrs[0].values).to.include(initialDept);
26682700

26692701
await request
2670-
.post(`${v1}/abac/users/sync`)
2702+
.post(api('abac/users/sync'))
26712703
.set(credentials)
26722704
.send({
26732705
usernames: ['david.scott', 'sergei.krikalev'],
@@ -3424,6 +3456,118 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
34243456
});
34253457
});
34263458

3459+
describe('Re-evaluation via POST /abac/users/sync', () => {
3460+
describe('PDP DENY removes the synced user', () => {
3461+
let room: IRoom;
3462+
let user: IUser;
3463+
const username = `abac-sync-deny-${Date.now()}`;
3464+
const email = `${username}@rocket.chat`;
3465+
3466+
before(async function () {
3467+
this.timeout(15000);
3468+
3469+
user = await createUser({ username, email });
3470+
room = (await createRoom({ type: 'p', name: `extpdp-sync-deny-${Date.now()}` })).body.group;
3471+
await request
3472+
.post(api('groups.invite'))
3473+
.set(credentials)
3474+
.send({ roomId: room._id, usernames: [user.username] })
3475+
.expect(200);
3476+
3477+
await mockServerReset();
3478+
await seedDefaultMocks();
3479+
await seedBulkDecisionByEntity([adminEmail, email], 'DECISION_DENY');
3480+
3481+
await request
3482+
.post(api(`abac/rooms/${room._id}/attributes/${attrKey}`))
3483+
.set(credentials)
3484+
.send({ values: ['alpha'] })
3485+
.expect(200);
3486+
});
3487+
3488+
after(async () => {
3489+
await Promise.all([deleteRoom({ type: 'p', roomId: room._id }), deleteUser(user)]);
3490+
});
3491+
3492+
it('keeps the user before re-evaluation', async () => {
3493+
const res = await request.get(api('groups.members')).set(credentials).query({ roomId: room._id }).expect(200);
3494+
const usernames = res.body.members.map((m: IUser) => m.username);
3495+
expect(usernames).to.include(user.username);
3496+
});
3497+
3498+
it('removes the user when the Virtru PDP returns DENY', async () => {
3499+
await mockServerReset();
3500+
await seedDefaultMocks();
3501+
await seedBulkDecisionByEntity([adminEmail], 'DECISION_DENY');
3502+
3503+
await request
3504+
.post(api('abac/users/sync'))
3505+
.set(credentials)
3506+
.send({ usernames: [user.username] })
3507+
.expect(200);
3508+
3509+
const res = await request.get(api('groups.members')).set(credentials).query({ roomId: room._id }).expect(200);
3510+
const usernames = res.body.members.map((m: IUser) => m.username);
3511+
expect(usernames).to.not.include(user.username);
3512+
});
3513+
3514+
it('keeps the room creator (permitted) after re-evaluation', async () => {
3515+
const res = await request.get(api('groups.members')).set(credentials).query({ roomId: room._id }).expect(200);
3516+
const memberIds = res.body.members.map((m: IUser) => m._id);
3517+
expect(memberIds).to.include(credentials['X-User-Id']);
3518+
});
3519+
});
3520+
3521+
describe('PDP PERMIT keeps the synced user', () => {
3522+
let room: IRoom;
3523+
let user: IUser;
3524+
const username = `abac-sync-permit-${Date.now()}`;
3525+
const email = `${username}@rocket.chat`;
3526+
3527+
before(async function () {
3528+
this.timeout(15000);
3529+
3530+
user = await createUser({ username, email });
3531+
room = (await createRoom({ type: 'p', name: `extpdp-sync-permit-${Date.now()}` })).body.group;
3532+
await request
3533+
.post(api('groups.invite'))
3534+
.set(credentials)
3535+
.send({ roomId: room._id, usernames: [user.username] })
3536+
.expect(200);
3537+
3538+
await mockServerReset();
3539+
await seedDefaultMocks();
3540+
await seedBulkDecisionByEntity([adminEmail, email], 'DECISION_DENY');
3541+
3542+
await request
3543+
.post(api(`abac/rooms/${room._id}/attributes/${attrKey}`))
3544+
.set(credentials)
3545+
.send({ values: ['alpha'] })
3546+
.expect(200);
3547+
});
3548+
3549+
after(async () => {
3550+
await Promise.all([deleteRoom({ type: 'p', roomId: room._id }), deleteUser(user)]);
3551+
});
3552+
3553+
it('keeps the user when the Virtru PDP returns PERMIT', async () => {
3554+
await mockServerReset();
3555+
await seedDefaultMocks();
3556+
await seedBulkDecisionByEntity([adminEmail, email], 'DECISION_DENY');
3557+
3558+
await request
3559+
.post(api('abac/users/sync'))
3560+
.set(credentials)
3561+
.send({ usernames: [user.username] })
3562+
.expect(200);
3563+
3564+
const res = await request.get(api('groups.members')).set(credentials).query({ roomId: room._id }).expect(200);
3565+
const usernames = res.body.members.map((m: IUser) => m.username);
3566+
expect(usernames).to.include(user.username);
3567+
});
3568+
});
3569+
});
3570+
34273571
describe('[GET] /abac/pdp/health', () => {
34283572
beforeEach(async () => {
34293573
await mockServerReset();
@@ -3945,7 +4089,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
39454089

39464090
it('POST /abac/users/sync is NOT blocked by external attribute store (no error-abac-attribute-store-external)', async () => {
39474091
const res = await request
3948-
.post(`${v1}/abac/users/sync`)
4092+
.post(api('abac/users/sync'))
39494093
.set(credentials)
39504094
.send({ usernames: ['no-such-user-vstore'] });
39514095
expect(res.body?.error).to.not.equal('error-abac-attribute-store-external');

‎ee/packages/abac/src/index.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type {
1212
AbacAuditReason,
1313
AbacAttributeStoreType,
1414
AbacPdpType,
15+
AbacUserIdentifiers,
1516
} from '@rocket.chat/core-typings';
1617
import { Rooms, AbacAttributes, Users, Subscriptions } from '@rocket.chat/models';
1718
import { escapeRegExp } from '@rocket.chat/string-helpers';
@@ -956,6 +957,30 @@ export class AbacService extends ServiceClass implements IAbacService {
956957
logger.error({ msg: 'Failed to evaluate room membership', err });
957958
}
958959
}
960+
961+
async reevaluateUsers(identifiers: AbacUserIdentifiers): Promise<void> {
962+
if (!this.pdp || !(await this.pdp.isAvailable())) {
963+
return;
964+
}
965+
966+
const users = await Users.findUsersByIdentifiers(identifiers, {
967+
projection: { _id: 1, emails: 1, username: 1, __rooms: 1 },
968+
}).toArray();
969+
970+
if (!users.length) {
971+
return;
972+
}
973+
974+
try {
975+
const nonCompliant = await this.pdp.reevaluateUsers(users);
976+
if (Array.isArray(nonCompliant) && nonCompliant.length) {
977+
await Promise.all(nonCompliant.map(({ user, room }) => limit(() => this.removeUserFromRoom(room, user as IUser, 'api'))));
978+
}
979+
} catch (err) {
980+
logger.error({ msg: 'Failed to reevaluate users', err });
981+
throw err;
982+
}
983+
}
959984
}
960985

961986
export { LocalPDP, VirtruPDP } from './pdp';

‎ee/packages/abac/src/pdp/LocalPDP.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1+
import { LDAPEnterprise } from '@rocket.chat/core-services';
12
import type { IAbacAttributeDefinition, IRoom, AtLeast, IUser } from '@rocket.chat/core-typings';
23
import { Rooms, Users } from '@rocket.chat/models';
34

45
import { OnlyCompliantCanBeAddedToRoomError } from '../errors';
56
import { buildCompliantConditions, buildNonCompliantConditions, buildRoomNonCompliantConditionsFromSubject } from '../helper';
6-
import type { IPolicyDecisionPoint } from './types';
7+
import type { IPolicyDecisionPoint, ReevaluationUser } from './types';
78

89
export class LocalPDP implements IPolicyDecisionPoint {
910
async isAvailable(): Promise<boolean> {
@@ -81,6 +82,10 @@ export class LocalPDP implements IPolicyDecisionPoint {
8182
throw new Error('evaluateUserRooms is not implemented for LocalPDP');
8283
}
8384

85+
async reevaluateUsers(users: ReevaluationUser[]): Promise<void> {
86+
await LDAPEnterprise.syncUsersAbacAttributesByIds(users.map((user) => user._id));
87+
}
88+
8489
async checkUsernamesMatchAttributes(usernames: string[], attributes: IAbacAttributeDefinition[], _object: IRoom): Promise<void> {
8590
const nonCompliantUsersFromList = await Users.find(
8691
{

0 commit comments

Comments
 (0)