diff --git a/ee/packages/abac/package.json b/ee/packages/abac/package.json index a9629ff00b994..9c6ea60fab5d9 100644 --- a/ee/packages/abac/package.json +++ b/ee/packages/abac/package.json @@ -25,6 +25,7 @@ "@rocket.chat/core-typings": "workspace:^", "@rocket.chat/logger": "workspace:^", "@rocket.chat/models": "workspace:^", + "mem": "^8.1.1", "mongodb": "6.10.0", "p-limit": "3.1.0" }, diff --git a/ee/packages/abac/src/errors.ts b/ee/packages/abac/src/errors.ts new file mode 100644 index 0000000000000..490ace6c9b7f1 --- /dev/null +++ b/ee/packages/abac/src/errors.ts @@ -0,0 +1,87 @@ +export enum AbacErrorCode { + InvalidAttributeValues = 'error-invalid-attribute-values', + InvalidAttributeKey = 'error-invalid-attribute-key', + AttributeNotFound = 'error-attribute-not-found', + AttributeInUse = 'error-attribute-in-use', + DuplicateAttributeKey = 'error-duplicate-attribute-key', + AttributeDefinitionNotFound = 'error-attribute-definition-not-found', + RoomNotFound = 'error-room-not-found', + CannotConvertDefaultRoomToAbac = 'error-cannot-convert-default-room-to-abac', + UsernamesNotMatchingAbacAttributes = 'error-usernames-not-matching-abac-attributes', + AbacUnsupportedObjectType = 'error-abac-unsupported-object-type', + AbacUnsupportedOperation = 'error-abac-unsupported-operation', +} + +export class AbacError extends Error { + public readonly code: AbacErrorCode; + + public readonly details?: unknown; + + constructor(code: AbacErrorCode, details?: unknown) { + super(code); + this.code = code; + this.details = details; + + Object.setPrototypeOf(this, new.target.prototype); + } +} + +export class AbacInvalidAttributeValuesError extends AbacError { + constructor(details?: unknown) { + super(AbacErrorCode.InvalidAttributeValues, details); + } +} + +export class AbacInvalidAttributeKeyError extends AbacError { + constructor(details?: unknown) { + super(AbacErrorCode.InvalidAttributeKey, details); + } +} + +export class AbacAttributeNotFoundError extends AbacError { + constructor(details?: unknown) { + super(AbacErrorCode.AttributeNotFound, details); + } +} + +export class AbacAttributeInUseError extends AbacError { + constructor(details?: unknown) { + super(AbacErrorCode.AttributeInUse, details); + } +} + +export class AbacDuplicateAttributeKeyError extends AbacError { + constructor(details?: unknown) { + super(AbacErrorCode.DuplicateAttributeKey, details); + } +} + +export class AbacAttributeDefinitionNotFoundError extends AbacError { + constructor(details?: unknown) { + super(AbacErrorCode.AttributeDefinitionNotFound, details); + } +} + +export class AbacRoomNotFoundError extends AbacError { + constructor(details?: unknown) { + super(AbacErrorCode.RoomNotFound, details); + } +} + +export class AbacCannotConvertDefaultRoomToAbacError extends AbacError { + constructor(details?: unknown) { + super(AbacErrorCode.CannotConvertDefaultRoomToAbac, details); + } +} + +export class AbacUnsupportedObjectTypeError extends AbacError { + constructor(details?: unknown) { + super(AbacErrorCode.AbacUnsupportedObjectType, details); + } +} + +export class AbacUnsupportedOperationError extends AbacError { + constructor(details?: unknown) { + super(AbacErrorCode.AbacUnsupportedOperation, details); + } +} diff --git a/ee/packages/abac/src/helper.spec.ts b/ee/packages/abac/src/helper.spec.ts index e81b9e409b1b0..75b53018f52cb 100644 --- a/ee/packages/abac/src/helper.spec.ts +++ b/ee/packages/abac/src/helper.spec.ts @@ -1,350 +1,191 @@ import { expect } from 'chai'; -import { didSubjectLoseAttributes } from './helper'; +import { + validateAndNormalizeAttributes, + MAX_ABAC_ATTRIBUTE_KEYS, + MAX_ABAC_ATTRIBUTE_VALUES, + diffAttributeSets, + buildRoomNonCompliantConditionsFromSubject, + extractAttribute, +} from './helper'; + +describe('validateAndNormalizeAttributes', () => { + it('normalizes keys and merges duplicate values from multiple entries', () => { + const result = validateAndNormalizeAttributes({ + ' dept ': [' sales ', 'marketing', 'sales', '', 'marketing '], + 'role': [' admin ', 'user', 'user '], + 'dept': ['engineering'], + }); -describe('didSubjectLoseAttributes', () => { - const call = (prev: { key: string; values: string[] }[], next: { key: string; values: string[] }[]) => - didSubjectLoseAttributes(prev, next); - - it('returns false if previous is empty (no attributes to lose)', () => { - const prev: { key: string; values: string[] }[] = []; - const next: { key: string; values: string[] }[] = [ - { - key: 'role', - values: ['admin'], - }, - ]; - - expect(call(prev, next)).to.be.false; + expect(result).to.deep.equal([ + { key: 'dept', values: ['sales', 'marketing', 'engineering'] }, + { key: 'role', values: ['admin', 'user'] }, + ]); }); - it('returns false if all previous attributes and values are preserved', () => { - const prev = [ - { - key: 'role', - values: ['admin', 'user'], - }, - { - key: 'dept', - values: ['sales'], - }, - ]; - - const next = [ - { - key: 'role', - values: ['admin', 'user'], - }, - { - key: 'dept', - values: ['sales'], - }, - ]; - - expect(call(prev, next)).to.be.false; + it('throws when a key has no remaining sanitized values', () => { + expect(() => + validateAndNormalizeAttributes({ + role: [' ', '\n', '\t'], + }), + ).to.throw('error-invalid-attribute-values'); }); - it('returns false if previous values are a subset of next values (nothing lost)', () => { - const prev = [ - { - key: 'role', - values: ['admin'], - }, - ]; - - const next = [ - { - key: 'role', - values: ['admin', 'user'], - }, - ]; - - expect(call(prev, next)).to.be.false; + it('throws when a key exceeds the maximum number of unique values', () => { + const values = Array.from({ length: MAX_ABAC_ATTRIBUTE_VALUES + 1 }, (_, i) => `value-${i}`); + expect(() => + validateAndNormalizeAttributes({ + role: values, + }), + ).to.throw('error-invalid-attribute-values'); }); - it('returns true if an entire previous attribute key is missing in next', () => { - const prev = [ - { - key: 'role', - values: ['admin'], - }, - { - key: 'dept', - values: ['sales'], - }, - ]; - - const next = [ - { - key: 'role', - values: ['admin'], - }, - ]; - - expect(call(prev, next)).to.be.true; + it('throws when total unique attribute keys exceeds limit', () => { + const attributes = Object.fromEntries(Array.from({ length: MAX_ABAC_ATTRIBUTE_KEYS + 1 }, (_, i) => [`key-${i}`, ['value']])); + expect(() => validateAndNormalizeAttributes(attributes)).to.throw('error-invalid-attribute-values'); }); +}); - it('returns true if any previous value is missing from corresponding next attribute', () => { - const prev = [ - { - key: 'role', - values: ['admin', 'user'], - }, - ]; - - const next = [ - { - key: 'role', - values: ['admin'], - }, - ]; +describe('diffAttributeSets', () => { + it('returns false/false when both sets are empty', () => { + const result = diffAttributeSets([], []); - expect(call(prev, next)).to.be.true; + expect(result).to.deep.equal({ added: false, removed: false }); }); - it('returns true if multiple attributes exist and one loses a value', () => { - const prev = [ - { - key: 'role', - values: ['admin', 'user'], - }, - { - key: 'dept', - values: ['sales'], - }, + it('detects only additions when moving from empty to non-empty', () => { + const current: { key: string; values: string[] }[] = []; + const next: { key: string; values: string[] }[] = [ + { key: 'dept', values: ['eng'] }, + { key: 'role', values: ['admin'] }, ]; - const next = [ - { - key: 'role', - values: ['admin'], - }, - { - key: 'dept', - values: ['sales'], - }, - ]; + const result = diffAttributeSets(current, next); - expect(call(prev, next)).to.be.true; + expect(result).to.deep.equal({ added: true, removed: false }); }); - it('returns true when next is empty but previous had attributes (all lost)', () => { - const prev = [ - { - key: 'role', - values: ['admin'], - }, + it('detects only removals when moving from non-empty to empty', () => { + const current: { key: string; values: string[] }[] = [ + { key: 'dept', values: ['eng'] }, + { key: 'role', values: ['admin'] }, ]; - const next: { key: string; values: string[] }[] = []; - expect(call(prev, next)).to.be.true; - }); + const result = diffAttributeSets(current, next); - it('returns true if one attribute key remains but all its values are lost', () => { - const prev = [ - { - key: 'role', - values: ['admin'], - }, - ]; - - const next = [ - { - key: 'role', - values: [], - }, - ]; - - expect(call(prev, next)).to.be.true; + expect(result).to.deep.equal({ added: false, removed: true }); }); - it('returns true on first detected loss even if multiple losses exist', () => { - const prev = [ - { - key: 'role', - values: ['admin', 'user'], - }, - { - key: 'dept', - values: ['sales', 'marketing'], - }, + it('returns false/false when sets are identical (same keys and values)', () => { + const current: { key: string; values: string[] }[] = [ + { key: 'dept', values: ['eng', 'sales'] }, + { key: 'role', values: ['admin'] }, ]; - - const next = [ - { - key: 'role', - values: ['admin'], - }, - { - key: 'dept', - values: ['sales'], - }, + const next: { key: string; values: string[] }[] = [ + { key: 'dept', values: ['eng', 'sales'] }, + { key: 'role', values: ['admin'] }, ]; - expect(call(prev, next)).to.be.true; + const result = diffAttributeSets(current, next); + + expect(result).to.deep.equal({ added: false, removed: false }); }); - it('does not mutate input arrays (pure function)', () => { - const prev = [ - { - key: 'role', - values: ['admin', 'user'], - }, + it('ignores value ordering and still treats sets as identical', () => { + const current: { key: string; values: string[] }[] = [ + { key: 'dept', values: ['eng', 'sales'] }, + { key: 'role', values: ['admin', 'user'] }, ]; - - const next = [ - { - key: 'role', - values: ['admin'], - }, + const next: { key: string; values: string[] }[] = [ + { key: 'role', values: ['user', 'admin'] }, + { key: 'dept', values: ['sales', 'eng'] }, ]; - const prevClone = JSON.parse(JSON.stringify(prev)); - const nextClone = JSON.parse(JSON.stringify(next)); + const result = diffAttributeSets(current, next); - call(prev, next); - - expect(prev).to.deep.equal(prevClone); - expect(next).to.deep.equal(nextClone); + expect(result).to.deep.equal({ added: false, removed: false }); }); - it('returns false if ordering of values changes but values remain (order-insensitive)', () => { - const prev = [ - { - key: 'role', - values: ['admin', 'user'], - }, - ]; + it('detects added values for an existing key', () => { + const current: { key: string; values: string[] }[] = [{ key: 'dept', values: ['eng'] }]; + const next: { key: string; values: string[] }[] = [{ key: 'dept', values: ['eng', 'sales'] }]; - const next = [ - { - key: 'role', - values: ['user', 'admin'], - }, - ]; + const result = diffAttributeSets(current, next); - expect(call(prev, next)).to.be.false; + expect(result).to.deep.equal({ added: true, removed: false }); }); - it('returns true if previous attribute key replaced with a different key only', () => { - const prev = [ - { - key: 'role', - values: ['admin'], - }, - ]; + it('detects removed values for an existing key', () => { + const current: { key: string; values: string[] }[] = [{ key: 'dept', values: ['eng', 'sales'] }]; + const next: { key: string; values: string[] }[] = [{ key: 'dept', values: ['eng'] }]; - const next = [ - { - key: 'dept', - values: ['admin'], - }, - ]; + const result = diffAttributeSets(current, next); - expect(call(prev, next)).to.be.true; + expect(result).to.deep.equal({ added: false, removed: true }); }); - it('returns false when next adds a new attribute without removing previous ones', () => { - const prev = [ - { - key: 'role', - values: ['admin'], - }, - ]; + it('detects added and removed values on the same key', () => { + const current: { key: string; values: string[] }[] = [{ key: 'dept', values: ['eng', 'sales'] }]; + const next: { key: string; values: string[] }[] = [{ key: 'dept', values: ['support'] }]; - const next = [ - { - key: 'role', - values: ['admin'], - }, - { - key: 'dept', - values: ['sales'], - }, - ]; + const result = diffAttributeSets(current, next); - expect(call(prev, next)).to.be.false; + expect(result).to.deep.equal({ added: true, removed: true }); }); - it('returns false when attribute keys are reordered but unchanged', () => { - const prev = [ - { - key: 'role', - values: ['admin'], - }, - { - key: 'dept', - values: ['sales'], - }, + it('detects newly added keys as additions', () => { + const current: { key: string; values: string[] }[] = [{ key: 'dept', values: ['eng'] }]; + const next: { key: string; values: string[] }[] = [ + { key: 'dept', values: ['eng'] }, + { key: 'role', values: ['admin'] }, ]; - const next = [ - { - key: 'dept', - values: ['sales'], - }, - { - key: 'role', - values: ['admin'], - }, - ]; + const result = diffAttributeSets(current, next); - expect(call(prev, next)).to.be.false; + expect(result).to.deep.equal({ added: true, removed: false }); }); - it('returns false when duplicate values are present but no unique value is lost', () => { - const prev = [ - { - key: 'role', - values: ['admin', 'admin'], - }, + it('detects removed keys as removals', () => { + const current: { key: string; values: string[] }[] = [ + { key: 'dept', values: ['eng'] }, + { key: 'role', values: ['admin'] }, ]; + const next: { key: string; values: string[] }[] = [{ key: 'dept', values: ['eng'] }]; - const next = [ - { - key: 'role', - values: ['admin'], - }, - ]; + const result = diffAttributeSets(current, next); - expect(call(prev, next)).to.be.false; + expect(result).to.deep.equal({ added: false, removed: true }); }); - it('returns false when previous attribute has an empty values array (nothing to lose)', () => { - const prev = [ - { - key: 'role', - values: [], - }, + it('detects both added and removed keys across sets', () => { + const current: { key: string; values: string[] }[] = [ + { key: 'dept', values: ['eng'] }, + { key: 'region', values: ['us'] }, ]; - - const next = [ - { - key: 'role', - values: ['admin'], - }, + const next: { key: string; values: string[] }[] = [ + { key: 'dept', values: ['eng'] }, + { key: 'role', values: ['admin'] }, ]; - expect(call(prev, next)).to.be.false; + const result = diffAttributeSets(current, next); + + expect(result).to.deep.equal({ added: true, removed: true }); }); - it('returns true when duplicate previous values include one that is lost', () => { - const prev = [ - { - key: 'role', - values: ['admin', 'admin', 'user'], - }, + it('handles mixed changes: new key, removed key, added and removed values', () => { + const current: { key: string; values: string[] }[] = [ + { key: 'dept', values: ['eng', 'sales'] }, + { key: 'region', values: ['us', 'eu'] }, ]; - - const next = [ - { - key: 'role', - values: ['admin'], - }, + const next: { key: string; values: string[] }[] = [ + { key: 'dept', values: ['eng', 'support'] }, // sales removed, support added + { key: 'role', values: ['admin'] }, // new key ]; - expect(call(prev, next)).to.be.true; + const result = diffAttributeSets(current, next); + + expect(result).to.deep.equal({ added: true, removed: true }); }); }); @@ -423,3 +264,162 @@ describe('buildNonCompliantConditions', () => { ]); }); }); + +describe('buildRoomNonCompliantConditionsFromSubject', () => { + it('returns a single condition matching rooms with keys not in the subject set when subject has one key', () => { + const subject = [{ key: 'dept', values: ['eng'] }]; + + const result = buildRoomNonCompliantConditionsFromSubject(subject); + + expect(result).to.deep.equal([ + { + abacAttributes: { + $elemMatch: { + key: { $nin: ['dept'] }, + }, + }, + }, + { + abacAttributes: { + $elemMatch: { + key: 'dept', + values: { $elemMatch: { $nin: ['eng'] } }, + }, + }, + }, + ]); + }); + + it('builds key-$nin condition using all subject keys and per-key $nin for each attribute', () => { + const subject = [ + { key: 'dept', values: ['eng', 'sales'] }, + { key: 'role', values: ['admin'] }, + ]; + + const result = buildRoomNonCompliantConditionsFromSubject(subject); + + expect(result[0]).to.deep.equal({ + abacAttributes: { + $elemMatch: { + key: { $nin: ['dept', 'role'] }, + }, + }, + }); + + expect(result[1]).to.deep.equal({ + abacAttributes: { + $elemMatch: { + key: 'dept', + values: { $elemMatch: { $nin: ['eng', 'sales'] } }, + }, + }, + }); + + expect(result[2]).to.deep.equal({ + abacAttributes: { + $elemMatch: { + key: 'role', + values: { $elemMatch: { $nin: ['admin'] } }, + }, + }, + }); + }); + + it('returns only the key-$nin condition when subject has no values for keys (empty arrays)', () => { + const subject = [ + { key: 'dept', values: [] }, + { key: 'role', values: [] }, + ]; + + const result = buildRoomNonCompliantConditionsFromSubject(subject); + + expect(result[0]).to.deep.equal({ + abacAttributes: { + $elemMatch: { + key: { $nin: ['dept', 'role'] }, + }, + }, + }); + + expect(result[1]).to.deep.equal({ + abacAttributes: { + $elemMatch: { + key: 'dept', + values: { $elemMatch: { $nin: [] } }, + }, + }, + }); + + expect(result[2]).to.deep.equal({ + abacAttributes: { + $elemMatch: { + key: 'role', + values: { $elemMatch: { $nin: [] } }, + }, + }, + }); + }); +}); + +describe('extractAttribute', () => { + it('returns undefined when ldapKey or abacKey is missing', () => { + const ldapUser = { memberOf: ['CN=Eng,OU=Groups'] } as any; + + expect(extractAttribute(ldapUser, '', 'dept')).to.be.undefined; + expect(extractAttribute(ldapUser, 'memberOf', '')).to.be.undefined; + }); + + it('returns undefined when ldapUser does not have the provided key', () => { + const ldapUser = { other: ['value'] } as any; + + expect(extractAttribute(ldapUser, 'memberOf', 'dept')).to.be.undefined; + }); + + it('extracts and normalizes a single string value', () => { + const ldapUser = { department: ' Engineering ' } as any; + + const result = extractAttribute(ldapUser, 'department', 'dept'); + + expect(result).to.deep.equal({ + key: 'dept', + values: ['Engineering'], + }); + }); + + it('extracts from an array, trimming and deduplicating values', () => { + const ldapUser = { + memberOf: [' Eng ', 'Sales', 'Eng', ' ', '\tSales\t'], + } as any; + + const result = extractAttribute(ldapUser, 'memberOf', 'dept'); + + // Order is preserved by insertion into the Set in implementation: ['Eng', 'Sales'] + expect(result).to.deep.equal({ + key: 'dept', + values: ['Eng', 'Sales'], + }); + }); + + it('ignores non-string values and empty/whitespace-only strings', () => { + const ldapUser = { + memberOf: [' ', 123, null, ' Eng ', undefined, {}, '\n'], + } as any; + + const result = extractAttribute(ldapUser, 'memberOf', 'dept'); + + expect(result).to.deep.equal({ + key: 'dept', + values: ['Eng'], + }); + }); + + it('returns undefined when all values are filtered out', () => { + const ldapUser = { + memberOf: [' ', '\n', '\t'], + } as any; + + const result = extractAttribute(ldapUser, 'memberOf', 'dept'); + + expect(result).to.be.undefined; + }); +}); diff --git a/ee/packages/abac/src/helper.ts b/ee/packages/abac/src/helper.ts index 733eeecd4b8b4..5522b09b99662 100644 --- a/ee/packages/abac/src/helper.ts +++ b/ee/packages/abac/src/helper.ts @@ -1,5 +1,17 @@ -import type { ILDAPEntry, IAbacAttributeDefinition } from '@rocket.chat/core-typings'; -import { AbacAttributes } from '@rocket.chat/models'; +import type { ILDAPEntry, IAbacAttributeDefinition, IRoom } from '@rocket.chat/core-typings'; +import { AbacAttributes, Rooms } from '@rocket.chat/models'; +import mem from 'mem'; + +import { + AbacAttributeDefinitionNotFoundError, + AbacCannotConvertDefaultRoomToAbacError, + AbacInvalidAttributeKeyError, + AbacInvalidAttributeValuesError, + AbacRoomNotFoundError, +} from './errors'; + +export const MAX_ABAC_ATTRIBUTE_KEYS = 10; +export const MAX_ABAC_ATTRIBUTE_VALUES = 10; export const extractAttribute = (ldapUser: ILDAPEntry, ldapKey: string, abacKey: string): IAbacAttributeDefinition | undefined => { if (!ldapKey || !abacKey) { @@ -78,81 +90,92 @@ export function diffAttributes(a: IAbacAttributeDefinition[] = [], b: IAbacAttri return diff; } -export function didAttributesChange(current: IAbacAttributeDefinition[], next: IAbacAttributeDefinition[]) { - let added = false; - const prevMap = new Map(current.map((a) => [a.key, new Set(a.values)])); - for (const { key, values } of next) { - const prevValues = prevMap.get(key); - if (!prevValues) { - added = true; - break; +export function validateAndNormalizeAttributes(attributes: Record): IAbacAttributeDefinition[] { + const keyPattern = /^[A-Za-z0-9_-]+$/; + const normalized: IAbacAttributeDefinition[] = []; + + const entries = Object.entries(attributes); + + const aggregated = new Map>(); + + for (const [rawKey, values] of entries) { + const key = rawKey.trim(); + + if (!key.length || !keyPattern.test(key)) { + throw new AbacInvalidAttributeKeyError(); } - for (const v of values) { - if (!prevValues.has(v)) { - added = true; - break; + + const bucket = aggregated.get(key) ?? new Set(); + if (!aggregated.has(key)) { + if (aggregated.size >= MAX_ABAC_ATTRIBUTE_KEYS) { + throw new AbacInvalidAttributeValuesError(); } + aggregated.set(key, bucket); } - if (added) { - break; + + for (const value of values) { + if (typeof value !== 'string') { + continue; + } + const trimmed = value.trim(); + if (!trimmed.length) { + continue; + } + if (bucket.size >= MAX_ABAC_ATTRIBUTE_VALUES && !bucket.has(trimmed)) { + throw new AbacInvalidAttributeValuesError(); + } + bucket.add(trimmed); } } - return added; -} - -export function validateAndNormalizeAttributes(attributes: Record): IAbacAttributeDefinition[] { - const keyPattern = /^[A-Za-z0-9_-]+$/; - const normalized: IAbacAttributeDefinition[] = []; - - if (Object.keys(attributes).length > 10) { - throw new Error('error-invalid-attribute-values'); + if (aggregated.size > MAX_ABAC_ATTRIBUTE_KEYS) { + throw new AbacInvalidAttributeValuesError(); } - for (const [key, values] of Object.entries(attributes)) { - if (!keyPattern.test(key)) { - throw new Error('error-invalid-attribute-key'); - } - if (values.length > 10) { - throw new Error('error-invalid-attribute-values'); + for (const [key, valueSet] of aggregated.entries()) { + if (!valueSet.size) { + throw new AbacInvalidAttributeValuesError(); } - normalized.push({ key, values }); + normalized.push({ key, values: Array.from(valueSet) }); } return normalized; } +const getAttributeDefinitionsFromDb = async (keys: string[]) => + AbacAttributes.find({ key: { $in: keys } }, { projection: { key: 1, values: 1 } }).toArray(); + +const getAttributeDefinitionsCached = mem(getAttributeDefinitionsFromDb, { + maxAge: 30_000, + cacheKey: JSON.stringify, +}); + export async function ensureAttributeDefinitionsExist(normalized: IAbacAttributeDefinition[]): Promise { if (!normalized.length) { return; } - const keys = normalized.map((a) => a.key); - const attributeDefinitions = await AbacAttributes.find({ key: { $in: keys } }, { projection: { key: 1, values: 1 } }).toArray(); + const uniqueKeys = [...new Set(normalized.map((a) => a.key))]; + const attributeDefinitions = await getAttributeDefinitionsCached(uniqueKeys); - const definitionValuesMap = new Map>(attributeDefinitions.map((def: any) => [def.key, new Set(def.values)])); - if (definitionValuesMap.size !== keys.length) { - throw new Error('error-attribute-definition-not-found'); + const definitionValuesMap = new Map>(attributeDefinitions.map((def) => [def.key, new Set(def.values)])); + if (definitionValuesMap.size !== uniqueKeys.length) { + throw new AbacAttributeDefinitionNotFoundError(); } for (const a of normalized) { const allowed = definitionValuesMap.get(a.key); if (!allowed) { - throw new Error('error-attribute-definition-not-found'); + throw new AbacAttributeDefinitionNotFoundError(); } for (const v of a.values) { if (!allowed.has(v)) { - throw new Error('error-invalid-attribute-values'); + throw new AbacInvalidAttributeValuesError(); } } } } -export function wereAttributeValuesAdded(prevValues: string[], newValues: string[]) { - const prevSet = new Set(prevValues); - return newValues.some((v) => !prevSet.has(v)); -} - export function buildNonCompliantConditions(newAttributes: IAbacAttributeDefinition[]) { return newAttributes.map(({ key, values }) => ({ abacAttributes: { @@ -177,25 +200,6 @@ export function buildCompliantConditions(attributes: IAbacAttributeDefinition[]) })); } -export function didSubjectLoseAttributes(previous: IAbacAttributeDefinition[], next: IAbacAttributeDefinition[]): boolean { - if (!previous.length) { - return false; - } - const nextMap = new Map(next.map((a) => [a.key, new Set(a.values)])); - for (const prevAttr of previous) { - const nextValues = nextMap.get(prevAttr.key); - if (!nextValues) { - return true; - } - for (const v of prevAttr.values) { - if (!nextValues.has(v)) { - return true; - } - } - } - return false; -} - export function buildRoomNonCompliantConditionsFromSubject(subjectAttributes: IAbacAttributeDefinition[]) { const map = new Map(subjectAttributes.map((a) => [a.key, new Set(a.values)])); const userKeys = Array.from(map.keys()); @@ -220,3 +224,101 @@ export function buildRoomNonCompliantConditionsFromSubject(subjectAttributes: IA } return conditions; } + +export async function getAbacRoom( + rid: string, +): Promise> { + const room = await Rooms.findOneByIdAndType< + Pick + >(rid, 'p', { + projection: { abacAttributes: 1, t: 1, teamMain: 1, teamDefault: 1, default: 1, name: 1 }, + }); + if (!room) { + throw new AbacRoomNotFoundError(); + } + if (room.default || room.teamDefault) { + throw new AbacCannotConvertDefaultRoomToAbacError(); + } + + return room; +} + +export function diffAttributeSets( + current: IAbacAttributeDefinition[] = [], + next: IAbacAttributeDefinition[] = [], +): { added: boolean; removed: boolean } { + const currentMap = new Map>(current.map((attr) => [attr.key, new Set(attr.values)])); + const nextMap = new Map>(next.map((attr) => [attr.key, new Set(attr.values)])); + + let added = false; + let removed = false; + + // Check removals and per-key value changes + for (const [key, currentValues] of currentMap) { + const nextValues = nextMap.get(key); + + if (!nextValues) { + // Key was completely removed + removed = true; + } else { + // Check removed values + for (const v of currentValues) { + if (!nextValues.has(v)) { + removed = true; + break; + } + } + } + + if (removed) { + break; + } + } + + // Check additions (new keys or new values on existing keys) + if (!removed) { + for (const [key, nextValues] of nextMap) { + const currentValues = currentMap.get(key); + + if (!currentValues) { + // New key added + added = true; + break; + } + + for (const v of nextValues) { + if (!currentValues.has(v)) { + added = true; + break; + } + } + + if (added) { + break; + } + } + } else { + // Even if we've already seen removals, we might still want to know if additions happened too + for (const [key, nextValues] of nextMap) { + const currentValues = currentMap.get(key); + + if (!currentValues) { + added = true; + break; + } + + for (const v of nextValues) { + if (!currentValues.has(v)) { + added = true; + break; + } + } + + if (added) { + break; + } + } + } + + return { added, removed }; +} diff --git a/ee/packages/abac/src/index.ts b/ee/packages/abac/src/index.ts index b220dbf19e47d..1b6e4bcbd3e3f 100644 --- a/ee/packages/abac/src/index.ts +++ b/ee/packages/abac/src/index.ts @@ -1,7 +1,7 @@ import { MeteorError, Room, ServiceClass } from '@rocket.chat/core-services'; import type { AbacActor, IAbacService } from '@rocket.chat/core-services'; import { AbacAccessOperation, AbacObjectType } from '@rocket.chat/core-typings'; -import type { IAbacAttribute, IAbacAttributeDefinition, IRoom, AtLeast, IUser, ILDAPEntry } from '@rocket.chat/core-typings'; +import type { IAbacAttribute, IAbacAttributeDefinition, IRoom, AtLeast, IUser, ILDAPEntry, ISubscription } from '@rocket.chat/core-typings'; import { Logger } from '@rocket.chat/logger'; import { Rooms, AbacAttributes, Users, Subscriptions, Settings } from '@rocket.chat/models'; import { escapeRegExp } from '@rocket.chat/string-helpers'; @@ -10,16 +10,24 @@ import pLimit from 'p-limit'; import { Audit } from './audit'; import { + AbacAttributeInUseError, + AbacAttributeNotFoundError, + AbacDuplicateAttributeKeyError, + AbacInvalidAttributeValuesError, + AbacUnsupportedObjectTypeError, + AbacUnsupportedOperationError, +} from './errors'; +import { + getAbacRoom, diffAttributes, extractAttribute, - didAttributesChange, - didSubjectLoseAttributes, - wereAttributeValuesAdded, + diffAttributeSets, buildCompliantConditions, buildNonCompliantConditions, validateAndNormalizeAttributes, ensureAttributeDefinitionsExist, buildRoomNonCompliantConditionsFromSubject, + MAX_ABAC_ATTRIBUTE_KEYS, } from './helper'; // Limit concurrent user removals to avoid overloading the server with too many operations at once @@ -72,7 +80,7 @@ export class AbacService extends ServiceClass implements IAbacService { const finalUser = await Users.setAbacAttributesById(user._id, finalAttributes); - if (didSubjectLoseAttributes(user?.abacAttributes || [], finalAttributes)) { + if (diffAttributeSets(user?.abacAttributes || [], finalAttributes).removed) { await this.onSubjectAttributesChanged(finalUser!, finalAttributes); } @@ -84,7 +92,7 @@ export class AbacService extends ServiceClass implements IAbacService { async addAbacAttribute(attribute: IAbacAttributeDefinition, actor: AbacActor): Promise { if (!attribute.values.length) { - throw new Error('error-invalid-attribute-values'); + throw new AbacInvalidAttributeValuesError(); } try { @@ -92,7 +100,7 @@ export class AbacService extends ServiceClass implements IAbacService { void Audit.attributeCreated(attribute, actor); } catch (e) { if (e instanceof Error && e.message.includes('E11000')) { - throw new Error('error-duplicate-attribute-key'); + throw new AbacDuplicateAttributeKeyError(); } throw e; } @@ -204,11 +212,11 @@ export class AbacService extends ServiceClass implements IAbacService { const existing = await AbacAttributes.findOneById(_id, { projection: { key: 1, values: 1 } }); if (!existing) { - throw new Error('error-attribute-not-found'); + throw new AbacAttributeNotFoundError(); } if (update.values && !update.values.length) { - throw new Error('error-invalid-attribute-values'); + throw new AbacInvalidAttributeValuesError(); } const newKey = update.key ?? existing.key; @@ -222,7 +230,7 @@ export class AbacService extends ServiceClass implements IAbacService { if (keyChanged || valuesToCheck.length) { const inUse = await Rooms.isAbacAttributeInUse(existing.key, valuesToCheck.length ? valuesToCheck : existing.values); if (inUse) { - throw new Error('error-attribute-in-use'); + throw new AbacAttributeInUseError(); } } @@ -243,7 +251,7 @@ export class AbacService extends ServiceClass implements IAbacService { void Audit.attributeUpdated(existing, modifier as IAbacAttributeDefinition, actor); } catch (e) { if (e instanceof Error && e.message.includes('E11000')) { - throw new Error('error-duplicate-attribute-key'); + throw new AbacDuplicateAttributeKeyError(); } throw e; } @@ -252,12 +260,12 @@ export class AbacService extends ServiceClass implements IAbacService { async deleteAbacAttributeById(_id: string, actor: AbacActor): Promise { const existing = await AbacAttributes.findOneById(_id, { projection: { key: 1, values: 1 } }); if (!existing) { - throw new Error('error-attribute-not-found'); + throw new AbacAttributeNotFoundError(); } const inUse = await Rooms.isAbacAttributeInUse(existing.key, existing.values); if (inUse) { - throw new Error('error-attribute-in-use'); + throw new AbacAttributeInUseError(); } await AbacAttributes.removeById(_id); @@ -267,7 +275,7 @@ export class AbacService extends ServiceClass implements IAbacService { async getAbacAttributeById(_id: string): Promise<{ key: string; values: string[]; usage: Record }> { const attribute = await AbacAttributes.findOneById(_id, { projection: { key: 1, values: 1 } }); if (!attribute) { - throw new Error('error-attribute-not-found'); + throw new AbacAttributeNotFoundError(); } const usageEntries = await Promise.all( @@ -294,17 +302,7 @@ export class AbacService extends ServiceClass implements IAbacService { } async setRoomAbacAttributes(rid: string, attributes: Record, actor: AbacActor): Promise { - const room = await Rooms.findOneByIdAndType< - Pick - >(rid, 'p', { - projection: { abacAttributes: 1, t: 1, teamMain: 1, teamDefault: 1, default: 1, name: 1 }, - }); - if (!room) { - throw new Error('error-room-not-found'); - } - if (room.default || room.teamDefault) { - throw new Error('error-cannot-convert-default-room-to-abac'); - } + const room = await getAbacRoom(rid); if (!Object.keys(attributes).length && room.abacAttributes?.length) { await Rooms.unsetAbacAttributesById(rid); @@ -320,31 +318,20 @@ export class AbacService extends ServiceClass implements IAbacService { void Audit.objectAttributeChanged({ _id: room._id, name: room.name }, room.abacAttributes || [], normalized, 'updated', actor); const previous: IAbacAttributeDefinition[] = room.abacAttributes || []; - if (didAttributesChange(previous, normalized)) { + if (diffAttributeSets(previous, normalized).added) { await this.onRoomAttributesChanged(room, (updated?.abacAttributes as IAbacAttributeDefinition[] | undefined) ?? normalized); } } async updateRoomAbacAttributeValues(rid: string, key: string, values: string[], actor: AbacActor): Promise { - const room = await Rooms.findOneByIdAndType< - Pick - >(rid, 'p', { - projection: { abacAttributes: 1, t: 1, teamMain: 1, teamDefault: 1, default: 1, name: 1 }, - }); - if (!room) { - throw new Error('error-room-not-found'); - } - - if (room.default || room.teamDefault) { - throw new Error('error-cannot-convert-default-room-to-abac'); - } + const room = await getAbacRoom(rid); const previous: IAbacAttributeDefinition[] = room.abacAttributes || []; const existingIndex = previous.findIndex((a) => a.key === key); const isNewKey = existingIndex === -1; - if (isNewKey && previous.length >= 10) { - throw new Error('error-invalid-attribute-values'); + if (isNewKey && previous.length >= MAX_ABAC_ATTRIBUTE_KEYS) { + throw new AbacInvalidAttributeValuesError(); } await ensureAttributeDefinitionsExist([{ key, values }]); @@ -379,23 +366,14 @@ export class AbacService extends ServiceClass implements IAbacService { actor, ); - if (wereAttributeValuesAdded(prevValues, values)) { + if (diffAttributeSets([previous[existingIndex]], [{ key, values }]).added) { const next = previous.map((a, i) => (i === existingIndex ? { key, values } : a)); await this.onRoomAttributesChanged(room, next); } } async removeRoomAbacAttribute(rid: string, key: string, actor: AbacActor): Promise { - const room = await Rooms.findOneByIdAndType>(rid, 'p', { - projection: { abacAttributes: 1, default: 1, teamDefault: 1, name: 1 }, - }); - if (!room) { - throw new Error('error-room-not-found'); - } - - if (room.default || room.teamDefault) { - throw new Error('error-cannot-convert-default-room-to-abac'); - } + const room = await getAbacRoom(rid); const previous: IAbacAttributeDefinition[] = room.abacAttributes || []; const exists = previous.some((a) => a.key === key); @@ -424,26 +402,15 @@ export class AbacService extends ServiceClass implements IAbacService { async addRoomAbacAttributeByKey(rid: string, key: string, values: string[], actor: AbacActor): Promise { await ensureAttributeDefinitionsExist([{ key, values }]); - const room = await Rooms.findOneByIdAndType< - Pick - >(rid, 'p', { - projection: { abacAttributes: 1, t: 1, teamMain: 1, teamDefault: 1, default: 1, name: 1 }, - }); - if (!room) { - throw new Error('error-room-not-found'); - } - - if (room.default || room.teamDefault) { - throw new Error('error-cannot-convert-default-room-to-abac'); - } + const room = await getAbacRoom(rid); const previous: IAbacAttributeDefinition[] = room.abacAttributes || []; if (previous.some((a) => a.key === key)) { - throw new Error('error-duplicate-attribute-key'); + throw new AbacDuplicateAttributeKeyError(); } - if (previous.length >= 10) { - throw new Error('error-invalid-attribute-values'); + if (previous.length >= MAX_ABAC_ATTRIBUTE_KEYS) { + throw new AbacInvalidAttributeValuesError(); } const updated = await Rooms.insertAbacAttributeIfNotExistsById(rid, key, values); @@ -457,24 +424,12 @@ export class AbacService extends ServiceClass implements IAbacService { async replaceRoomAbacAttributeByKey(rid: string, key: string, values: string[], actor: AbacActor): Promise { await ensureAttributeDefinitionsExist([{ key, values }]); - const room = await Rooms.findOneByIdAndType< - Pick - >(rid, 'p', { - projection: { abacAttributes: 1, t: 1, teamMain: 1, teamDefault: 1, default: 1, name: 1 }, - }); - if (!room) { - throw new Error('error-room-not-found'); - } - - if (room.default || room.teamDefault) { - throw new Error('error-cannot-convert-default-room-to-abac'); - } + const room = await getAbacRoom(rid); - const exists = room?.abacAttributes?.some((a) => a.key === key); + const exists = room?.abacAttributes?.find((a) => a.key === key); if (exists) { const updated = await Rooms.updateAbacAttributeValuesArrayFilteredById(rid, key, values); - const prevValues = room.abacAttributes?.find((a) => a.key === key)?.values ?? []; void Audit.objectAttributeChanged( { _id: room._id, name: room.name }, @@ -483,15 +438,15 @@ export class AbacService extends ServiceClass implements IAbacService { 'key-updated', actor, ); - if (wereAttributeValuesAdded(prevValues, values)) { + if (diffAttributeSets([exists], [{ key, values }]).added) { await this.onRoomAttributesChanged(room, updated?.abacAttributes || []); } return; } - if (room?.abacAttributes?.length === 10) { - throw new Error('error-invalid-attribute-values'); + if (room?.abacAttributes?.length === MAX_ABAC_ATTRIBUTE_KEYS) { + throw new AbacInvalidAttributeValuesError(); } const updated = await Rooms.insertAbacAttributeIfNotExistsById(rid, key, values); @@ -537,6 +492,19 @@ export class AbacService extends ServiceClass implements IAbacService { }); } + private shouldUseCache(decisionCacheTimeout: number, userSub: ISubscription) { + // Cases: + // 1) Never checked before -> check now + // 2) Checked before, but cache expired -> check now + // 3) Checked before, and cache valid -> use cached decision (subsciprtion exists) + // 4) Cache disabled (0) -> always check + return ( + decisionCacheTimeout > 0 && + userSub.abacLastTimeChecked && + Date.now() - userSub.abacLastTimeChecked.getTime() < decisionCacheTimeout * 1000 + ); + } + async canAccessObject( room: Pick, user: Pick, @@ -545,11 +513,11 @@ export class AbacService extends ServiceClass implements IAbacService { ) { // We may need this flex for phase 2, but for now only ROOM/READ is supported if (objectType !== AbacObjectType.ROOM) { - throw new Error('error-abac-unsupported-object-type'); + throw new AbacUnsupportedObjectTypeError(); } if (action !== AbacAccessOperation.READ) { - throw new Error('error-abac-unsupported-operation'); + throw new AbacUnsupportedOperationError(); } if (!user?._id || !room?.abacAttributes?.length) { @@ -562,16 +530,7 @@ export class AbacService extends ServiceClass implements IAbacService { return false; } - // Cases: - // 1) Never checked before -> check now - // 2) Checked before, but cache expired -> check now - // 3) Checked before, and cache valid -> use cached decision (subsciprtion exists) - // 4) Cache disabled (0) -> always check - if ( - decisionCacheTimeout > 0 && - userSub.abacLastTimeChecked && - Date.now() - userSub.abacLastTimeChecked.getTime() < decisionCacheTimeout * 1000 - ) { + if (this.shouldUseCache(decisionCacheTimeout, userSub)) { this.logger.debug({ msg: 'Using cached ABAC decision', userId: user._id, roomId: room._id }); return !!userSub; } @@ -641,7 +600,15 @@ export class AbacService extends ServiceClass implements IAbacService { Room.removeUserFromRoom(rid, doc, { skipAppPreEvents: true, customSystemMessage: 'abac-removed-user-from-room' as const, - }).then(() => void Audit.actionPerformed({ _id: doc._id, username: doc.username }, { _id: rid }, 'room-attributes-change')), + }) + .then(() => void Audit.actionPerformed({ _id: doc._id, username: doc.username }, { _id: rid }, 'room-attributes-change')) + .catch((err) => { + this.logger.error({ + msg: 'Failed to remove user from ABAC room after room attributes changed', + rid, + err, + }); + }), ), ); } @@ -685,7 +652,15 @@ export class AbacService extends ServiceClass implements IAbacService { Room.removeUserFromRoom(room._id, user, { skipAppPreEvents: true, customSystemMessage: 'abac-removed-user-from-room' as const, - }).then(() => void Audit.actionPerformed({ _id: user._id, username: user.username }, { _id: room._id }, 'ldap-sync')), + }) + .then(() => void Audit.actionPerformed({ _id: user._id, username: user.username }, { _id: room._id }, 'ldap-sync')) + .catch((err) => { + this.logger.error({ + msg: 'Failed to remove user from ABAC room after room attributes changed', + rid: room._id, + err, + }); + }), ), ); } @@ -708,7 +683,15 @@ export class AbacService extends ServiceClass implements IAbacService { Room.removeUserFromRoom(room._id, user, { skipAppPreEvents: true, customSystemMessage: 'abac-removed-user-from-room' as const, - }).then(() => void Audit.actionPerformed({ _id: user._id, username: user.username }, { _id: room._id }, 'ldap-sync')), + }) + .then(() => void Audit.actionPerformed({ _id: user._id, username: user.username }, { _id: room._id }, 'ldap-sync')) + .catch((err) => { + this.logger.error({ + msg: 'Failed to remove user from ABAC room after room attributes changed', + rid: room._id, + err, + }); + }), ), ); } diff --git a/ee/packages/abac/src/service.spec.ts b/ee/packages/abac/src/service.spec.ts index 111f44a78a488..9b67209b5349f 100644 --- a/ee/packages/abac/src/service.spec.ts +++ b/ee/packages/abac/src/service.spec.ts @@ -69,6 +69,10 @@ jest.mock('@rocket.chat/core-services', () => { }; }); +jest.mock('mem', () => { + return jest.fn((fn: any) => fn); +}); + describe('AbacService (unit)', () => { let service: AbacService; @@ -603,11 +607,9 @@ describe('AbacService (unit)', () => { expect(mockSetAbacAttributesById).not.toHaveBeenCalled(); }); - it('throws error-attribute-definition-not-found for empty value array', async () => { + it('throws error-invalid-attribute-values for empty value array', async () => { mockFindOneByIdAndType.mockResolvedValueOnce({ _id: 'r1', abacAttributes: [] }); - await expect(service.setRoomAbacAttributes('r1', { dept: [] as any }, fakeActor)).rejects.toThrow( - 'error-attribute-definition-not-found', - ); + await expect(service.setRoomAbacAttributes('r1', { dept: [] as any }, fakeActor)).rejects.toThrow('error-invalid-attribute-values'); expect(mockSetAbacAttributesById).not.toHaveBeenCalled(); }); @@ -632,18 +634,15 @@ describe('AbacService (unit)', () => { expect(mockSetAbacAttributesById).not.toHaveBeenCalled(); }); - it('sets attributes for new key (with duplicate values) and calls hook', async () => { - mockFindOneByIdAndType.mockResolvedValueOnce({ _id: 'r1', abacAttributes: [] }); + it('does not call onroomattributechanged when the change is a duplicated attribute value', async () => { + mockFindOneByIdAndType.mockResolvedValueOnce({ _id: 'r1', abacAttributes: [{ key: 'dept', values: ['eng', 'sales'] }] }); mockAbacFind.mockReturnValueOnce({ toArray: async () => [{ key: 'dept', values: ['eng', 'sales'] }], }); await service.setRoomAbacAttributes('r1', { dept: ['eng', 'eng', 'sales'] }, fakeActor); - expect(mockSetAbacAttributesById).toHaveBeenCalledWith('r1', [{ key: 'dept', values: ['eng', 'eng', 'sales'] }]); - expect((service as any).onRoomAttributesChanged).toHaveBeenCalledWith(expect.objectContaining({ _id: 'r1' }), [ - { key: 'dept', values: ['eng', 'eng', 'sales'] }, - ]); + expect((service as any).onRoomAttributesChanged).not.toHaveBeenCalled(); }); it('does not call onRoomAttributesChanged when an existing value is removed', async () => { diff --git a/yarn.lock b/yarn.lock index 1ebeea37ba9d1..a26dcb758b62d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8287,6 +8287,7 @@ __metadata: "@types/node": "npm:~22.16.1" eslint: "npm:~8.45.0" jest: "npm:~30.0.5" + mem: "npm:^8.1.1" mongodb: "npm:6.10.0" mongodb-memory-server: "npm:^10.1.4" p-limit: "npm:3.1.0"