Skip to content

chore: Delegate user sync to PDP #40836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dionisio-bot merged 15 commits into from
Jun 9, 2026
Merged

chore: Delegate user sync to PDP #40836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
3154908
feat(abac): add reevaluateUsers PDP contract and identifier types
KevLehman Jun 5, 2026
33aeca0
feat(ldap-enterprise): add id-based ABAC sync and core-services proxy
KevLehman Jun 5, 2026
22914f2
feat(abac): implement VirtruPDP.reevaluateUsers
KevLehman Jun 5, 2026
1187842
feat(abac): implement LocalPDP.reevaluateUsers via ldap-enterprise br…
KevLehman Jun 5, 2026
475cb3a
feat(abac): add AbacService.reevaluateUsers dispatch
KevLehman Jun 5, 2026
4493fbd
feat(abac): route abac/users/sync through PDP reevaluation
KevLehman Jun 5, 2026
de9e5ca
test(abac): cover strategy-agnostic abac/users/sync endpoint
KevLehman Jun 5, 2026
3a820f3
style(abac): lint/format reevaluateUsers signatures
KevLehman Jun 5, 2026
8bba69c
test(abac): use non-empty identifiers in abac/users/sync e2e tests
KevLehman Jun 5, 2026
5e6243d
test(abac): cover Virtru PDP re-evaluation via abac/users/sync
KevLehman Jun 8, 2026
be7a697
test(abac): use api() base-path helper for sync/re-evaluation requests
KevLehman Jun 8, 2026
eb95f27
refactor(abac): let PDP.reevaluateUsers return void when it self-hand…
KevLehman Jun 8, 2026
0a26692
fix(abac): re-throw reevaluateUsers failures so abac/users/sync surfa…
KevLehman Jun 8, 2026
f10526b
review
KevLehman Jun 9, 2026
5230be5
test
KevLehman Jun 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions apps/meteor/ee/server/api/abac/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { AbacAttributeStoreExternalError, getPdpHealthErrorCode } from '@rocket.chat/abac';
import { Abac, LDAPEnterprise } from '@rocket.chat/core-services';
import { Abac } from '@rocket.chat/core-services';
import type { AbacActor } from '@rocket.chat/core-services';
import type { IServerEvents, IUser } from '@rocket.chat/core-typings';
import { ServerEvents, Users } from '@rocket.chat/models';
import { ServerEvents } from '@rocket.chat/models';
import { validateUnauthorizedErrorResponse } from '@rocket.chat/rest-typings/src/v1/Ajv';
import { convertSubObjectsIntoPaths } from '@rocket.chat/tools';

Expand Down Expand Up @@ -209,7 +209,7 @@ const abacEndpoints = API.v1
{
authRequired: true,
permissionsRequired: ['abac-management', 'manage-abac-admin-room-attributes'],
license: ['abac', 'ldap-enterprise'],
license: ['abac'],
body: POSTAbacUsersSyncBodySchema,
response: {
200: GenericSuccessSchema,
Expand All @@ -225,7 +225,7 @@ const abacEndpoints = API.v1

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

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

return API.v1.success();
},
Expand Down
5 changes: 5 additions & 0 deletions apps/meteor/ee/server/local-services/ldap/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ServiceClassInternal, type ILDAPEEService } from '@rocket.chat/core-services';
import type { IUser } from '@rocket.chat/core-typings';
import { Users } from '@rocket.chat/models';
import type { FindCursor } from 'mongodb';

import { LDAPEEManager } from '../../lib/ldap/Manager';
Expand Down Expand Up @@ -30,4 +31,8 @@ export class LDAPEEService extends ServiceClassInternal implements ILDAPEEServic
async syncUsersAbacAttributes(users: FindCursor<IUser>): Promise<void> {
return LDAPEEManager.syncUsersAbacAttributes(users);
}

async syncUsersAbacAttributesByIds(userIds: string[]): Promise<void> {
return LDAPEEManager.syncUsersAbacAttributes(Users.findUsersByIdentifiers({ ids: userIds }));
}
}
164 changes: 154 additions & 10 deletions apps/meteor/tests/end-to-end/api/abac.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { expect } from 'chai';
import { before, after, describe, it } from 'mocha';
import { MongoClient } from 'mongodb';

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

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

it('POST /abac/users/sync should fail with error-abac-not-enabled', async () => {
await request
.post(api('abac/users/sync'))
.set(credentials)
.send({ ids: ['no-such-user-id'] })
.expect(400)
.expect((res) => {
expect(res.body.error).to.include('error-abac-not-enabled');
});
});

after(async () => {
await updateSetting('ABAC_Enabled', true);
});
Expand Down Expand Up @@ -1832,6 +1843,27 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
});
});

describe('POST /abac/users/sync (strategy-agnostic)', () => {
before(async () => {
await updateSetting('ABAC_Enabled', true);
});

after(async () => {
await updateSetting('ABAC_Enabled', false);
});

it('responds 200 with success:true when ABAC_Enabled=true and PDP type=local (no-match id)', async () => {
await request
.post(api('abac/users/sync'))
.set(credentials)
.send({ ids: ['no-such-user-id'] })
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
});
});
});

describe('Room access (invite, addition)', () => {
let roomWithoutAttr: IRoom;
let roomWithAttr: IRoom;
Expand Down Expand Up @@ -2503,7 +2535,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I

it('should sync ABAC attributes for SOME users via /abac/users/sync', async () => {
await request
.post(`${v1}/abac/users/sync`)
.post(api('abac/users/sync'))
.set(credentials)
.send({
usernames: ['david.scott', 'gene.cernan'],
Expand Down Expand Up @@ -2533,7 +2565,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
it('should fail /abac/users/sync when more than 100 usernames are provided', async () => {
const usernames = Array.from({ length: 101 }, (_, i) => `user_${i}@example.com`);
await request
.post(`${v1}/abac/users/sync`)
.post(api('abac/users/sync'))
.set(credentials)
.send({
usernames,
Expand All @@ -2547,7 +2579,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
it('should fail /abac/users/sync when more than 100 ids are provided', async () => {
const ids = Array.from({ length: 101 }, (_, i) => `id_${i}`);
await request
.post(`${v1}/abac/users/sync`)
.post(api('abac/users/sync'))
.set(credentials)
.send({
ids,
Expand All @@ -2561,7 +2593,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
it('should fail /abac/users/sync when more than 100 emails are provided', async () => {
const emails = Array.from({ length: 101 }, (_, i) => `user_${i}@example.com`);
await request
.post(`${v1}/abac/users/sync`)
.post(api('abac/users/sync'))
.set(credentials)
.send({
emails,
Expand All @@ -2575,7 +2607,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
it('should fail /abac/users/sync when more than 100 ldapIds are provided', async () => {
const ldapIds = Array.from({ length: 101 }, (_, i) => `ldap_${i}`);
await request
.post(`${v1}/abac/users/sync`)
.post(api('abac/users/sync'))
.set(credentials)
.send({
ldapIds,
Expand All @@ -2589,7 +2621,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
it('should succeed /abac/users/sync when exactly 100 usernames are provided (boundary)', async () => {
const usernames = Array.from({ length: 100 }, (_, i) => `boundary_user_${i}`);
await request
.post(`${v1}/abac/users/sync`)
.post(api('abac/users/sync'))
.set(credentials)
.send({
usernames,
Expand Down Expand Up @@ -2667,7 +2699,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
expect(sergeiInitialAttrs[0].values).to.include(initialDept);

await request
.post(`${v1}/abac/users/sync`)
.post(api('abac/users/sync'))
.set(credentials)
.send({
usernames: ['david.scott', 'sergei.krikalev'],
Expand Down Expand Up @@ -3424,6 +3456,118 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
});
});

describe('Re-evaluation via POST /abac/users/sync', () => {
describe('PDP DENY removes the synced user', () => {
let room: IRoom;
let user: IUser;
const username = `abac-sync-deny-${Date.now()}`;
const email = `${username}@rocket.chat`;

before(async function () {
this.timeout(15000);

user = await createUser({ username, email });
room = (await createRoom({ type: 'p', name: `extpdp-sync-deny-${Date.now()}` })).body.group;
await request
.post(api('groups.invite'))
.set(credentials)
.send({ roomId: room._id, usernames: [user.username] })
.expect(200);

await mockServerReset();
await seedDefaultMocks();
await seedBulkDecisionByEntity([adminEmail, email], 'DECISION_DENY');

await request
.post(api(`abac/rooms/${room._id}/attributes/${attrKey}`))
.set(credentials)
.send({ values: ['alpha'] })
.expect(200);
});

after(async () => {
await Promise.all([deleteRoom({ type: 'p', roomId: room._id }), deleteUser(user)]);
});

it('keeps the user before re-evaluation', async () => {
const res = await request.get(api('groups.members')).set(credentials).query({ roomId: room._id }).expect(200);
const usernames = res.body.members.map((m: IUser) => m.username);
expect(usernames).to.include(user.username);
});

it('removes the user when the Virtru PDP returns DENY', async () => {
await mockServerReset();
await seedDefaultMocks();
await seedBulkDecisionByEntity([adminEmail], 'DECISION_DENY');

await request
.post(api('abac/users/sync'))
.set(credentials)
.send({ usernames: [user.username] })
.expect(200);

const res = await request.get(api('groups.members')).set(credentials).query({ roomId: room._id }).expect(200);
const usernames = res.body.members.map((m: IUser) => m.username);
expect(usernames).to.not.include(user.username);
});

it('keeps the room creator (permitted) after re-evaluation', async () => {
const res = await request.get(api('groups.members')).set(credentials).query({ roomId: room._id }).expect(200);
const memberIds = res.body.members.map((m: IUser) => m._id);
expect(memberIds).to.include(credentials['X-User-Id']);
});
});

describe('PDP PERMIT keeps the synced user', () => {
let room: IRoom;
let user: IUser;
const username = `abac-sync-permit-${Date.now()}`;
const email = `${username}@rocket.chat`;

before(async function () {
this.timeout(15000);

user = await createUser({ username, email });
room = (await createRoom({ type: 'p', name: `extpdp-sync-permit-${Date.now()}` })).body.group;
await request
.post(api('groups.invite'))
.set(credentials)
.send({ roomId: room._id, usernames: [user.username] })
.expect(200);

await mockServerReset();
await seedDefaultMocks();
await seedBulkDecisionByEntity([adminEmail, email], 'DECISION_DENY');

await request
.post(api(`abac/rooms/${room._id}/attributes/${attrKey}`))
.set(credentials)
.send({ values: ['alpha'] })
.expect(200);
});

after(async () => {
await Promise.all([deleteRoom({ type: 'p', roomId: room._id }), deleteUser(user)]);
});

it('keeps the user when the Virtru PDP returns PERMIT', async () => {
await mockServerReset();
await seedDefaultMocks();
await seedBulkDecisionByEntity([adminEmail, email], 'DECISION_DENY');

await request
.post(api('abac/users/sync'))
.set(credentials)
.send({ usernames: [user.username] })
.expect(200);

const res = await request.get(api('groups.members')).set(credentials).query({ roomId: room._id }).expect(200);
const usernames = res.body.members.map((m: IUser) => m.username);
expect(usernames).to.include(user.username);
});
});
});

describe('[GET] /abac/pdp/health', () => {
beforeEach(async () => {
await mockServerReset();
Expand Down Expand Up @@ -3945,7 +4089,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I

it('POST /abac/users/sync is NOT blocked by external attribute store (no error-abac-attribute-store-external)', async () => {
const res = await request
.post(`${v1}/abac/users/sync`)
.post(api('abac/users/sync'))
.set(credentials)
.send({ usernames: ['no-such-user-vstore'] });
expect(res.body?.error).to.not.equal('error-abac-attribute-store-external');
Expand Down
25 changes: 25 additions & 0 deletions ee/packages/abac/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
AbacAuditReason,
AbacAttributeStoreType,
AbacPdpType,
AbacUserIdentifiers,
} from '@rocket.chat/core-typings';
import { Rooms, AbacAttributes, Users, Subscriptions } from '@rocket.chat/models';
import { escapeRegExp } from '@rocket.chat/string-helpers';
Expand Down Expand Up @@ -956,6 +957,30 @@ export class AbacService extends ServiceClass implements IAbacService {
logger.error({ msg: 'Failed to evaluate room membership', err });
}
}

async reevaluateUsers(identifiers: AbacUserIdentifiers): Promise<void> {
if (!this.pdp || !(await this.pdp.isAvailable())) {
return;
}

const users = await Users.findUsersByIdentifiers(identifiers, {
Comment thread
KevLehman marked this conversation as resolved.
projection: { _id: 1, emails: 1, username: 1, __rooms: 1 },
}).toArray();

if (!users.length) {
return;
}

try {
const nonCompliant = await this.pdp.reevaluateUsers(users);
if (Array.isArray(nonCompliant) && nonCompliant.length) {
await Promise.all(nonCompliant.map(({ user, room }) => limit(() => this.removeUserFromRoom(room, user as IUser, 'api'))));
}
} catch (err) {
Comment thread
KevLehman marked this conversation as resolved.
logger.error({ msg: 'Failed to reevaluate users', err });
throw err;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}

export { LocalPDP, VirtruPDP } from './pdp';
Expand Down
7 changes: 6 additions & 1 deletion ee/packages/abac/src/pdp/LocalPDP.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { LDAPEnterprise } from '@rocket.chat/core-services';
import type { IAbacAttributeDefinition, IRoom, AtLeast, IUser } from '@rocket.chat/core-typings';
import { Rooms, Users } from '@rocket.chat/models';

import { OnlyCompliantCanBeAddedToRoomError } from '../errors';
import { buildCompliantConditions, buildNonCompliantConditions, buildRoomNonCompliantConditionsFromSubject } from '../helper';
import type { IPolicyDecisionPoint } from './types';
import type { IPolicyDecisionPoint, ReevaluationUser } from './types';

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

async reevaluateUsers(users: ReevaluationUser[]): Promise<void> {
await LDAPEnterprise.syncUsersAbacAttributesByIds(users.map((user) => user._id));
}

async checkUsernamesMatchAttributes(usernames: string[], attributes: IAbacAttributeDefinition[], _object: IRoom): Promise<void> {
const nonCompliantUsersFromList = await Users.find(
{
Expand Down
Loading
Loading