Skip to content

Commit 5000d66

Browse files
authored
fix: Prevent attr update when external pdp is down (#39978)
1 parent fc39e07 commit 5000d66

File tree

4 files changed

+33
-8
lines changed

4 files changed

+33
-8
lines changed

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2942,6 +2942,21 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
29422942

29432943
await deleteUser(newUser);
29442944
});
2945+
2946+
it('should deny room attribute changes when PDP is unavailable', async () => {
2947+
await mockServerReset();
2948+
await mockServerSet('GET', '/healthz', { status: 'NOT_SERVING' });
2949+
2950+
await request
2951+
.post(`/api/v1/abac/rooms/${room._id}/attributes/${attrKey}`)
2952+
.set(credentials)
2953+
.send({ values: ['beta'] })
2954+
.expect(400)
2955+
.expect((res) => {
2956+
expect(res.body).to.have.property('success', false);
2957+
expect(res.body).to.have.property('error', 'error-pdp-unavailable');
2958+
});
2959+
});
29452960
});
29462961

29472962
describe('Selective DENY: only non-permitted users are removed', () => {

ee/packages/abac/src/errors.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export enum AbacErrorCode {
1010
AbacUnsupportedObjectType = 'error-abac-unsupported-object-type',
1111
AbacUnsupportedOperation = 'error-abac-unsupported-operation',
1212
OnlyCompliantCanBeAddedToRoom = 'error-only-compliant-users-can-be-added-to-abac-rooms',
13+
PdpUnavailable = 'error-pdp-unavailable',
1314
}
1415

1516
export class AbacError extends Error {
@@ -91,3 +92,9 @@ export class OnlyCompliantCanBeAddedToRoomError extends AbacError {
9192
super(AbacErrorCode.OnlyCompliantCanBeAddedToRoom, details);
9293
}
9394
}
95+
96+
export class PdpUnavailableError extends AbacError {
97+
constructor(details?: unknown) {
98+
super(AbacErrorCode.PdpUnavailable, details);
99+
}
100+
}

ee/packages/abac/src/index.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
AbacInvalidAttributeValuesError,
2525
AbacUnsupportedObjectTypeError,
2626
AbacUnsupportedOperationError,
27+
PdpUnavailableError,
2728
} from './errors';
2829
import {
2930
getAbacRoom,
@@ -416,6 +417,7 @@ export class AbacService extends ServiceClass implements IAbacService {
416417
}
417418

418419
async setRoomAbacAttributes(rid: string, attributes: Record<string, string[]>, actor: AbacActor): Promise<void> {
420+
await this.ensurePdpAvailable();
419421
const room = await getAbacRoom(rid);
420422

421423
if (!Object.keys(attributes).length && room.abacAttributes?.length) {
@@ -438,6 +440,7 @@ export class AbacService extends ServiceClass implements IAbacService {
438440
}
439441

440442
async updateRoomAbacAttributeValues(rid: string, key: string, values: string[], actor: AbacActor): Promise<void> {
443+
await this.ensurePdpAvailable();
441444
const room = await getAbacRoom(rid);
442445

443446
const previous: IAbacAttributeDefinition[] = room.abacAttributes || [];
@@ -514,6 +517,7 @@ export class AbacService extends ServiceClass implements IAbacService {
514517
}
515518

516519
async addRoomAbacAttributeByKey(rid: string, key: string, values: string[], actor: AbacActor): Promise<void> {
520+
await this.ensurePdpAvailable();
517521
await ensureAttributeDefinitionsExist([{ key, values }]);
518522

519523
const room = await getAbacRoom(rid);
@@ -536,6 +540,7 @@ export class AbacService extends ServiceClass implements IAbacService {
536540
}
537541

538542
async replaceRoomAbacAttributeByKey(rid: string, key: string, values: string[], actor: AbacActor): Promise<void> {
543+
await this.ensurePdpAvailable();
539544
await ensureAttributeDefinitionsExist([{ key, values }]);
540545

541546
const room = await getAbacRoom(rid);
@@ -649,6 +654,12 @@ export class AbacService extends ServiceClass implements IAbacService {
649654

650655
private pdpType: 'local' | 'virtru' = 'local';
651656

657+
private async ensurePdpAvailable(): Promise<void> {
658+
if (!(await this.pdp?.isAvailable())) {
659+
throw new PdpUnavailableError();
660+
}
661+
}
662+
652663
private async removeUserFromRoom(room: AtLeast<IRoom, '_id'>, user: IUser, reason: AbacAuditReason): Promise<void> {
653664
return Room.removeUserFromRoom(room._id, user, {
654665
skipAppPreEvents: true,

ee/packages/abac/src/pdp/VirtruPDP.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -297,14 +297,6 @@ export class VirtruPDP implements IPolicyDecisionPoint {
297297
return [];
298298
}
299299

300-
if (!(await this.isAvailable())) {
301-
pdpLogger.warn({
302-
msg: 'Virtru PDP is unavailable, skipping room attributes evaluation — no users will be removed',
303-
roomId: room._id,
304-
});
305-
return [];
306-
}
307-
308300
const users = Users.findActiveByRoomIds([room._id], {
309301
projection: { _id: 1, emails: 1, username: 1 },
310302
});

0 commit comments

Comments
 (0)