Chore: break LDAP manager into smaller pieces to improve unit tests (#26994)

* break LDAP manager into smaller pieces to improve unit tests

* Fix

* Apply suggestions from code review

Co-authored-by: Diego Sampaio <chinello@gmail.com>

* Apply suggestions from code review

Co-authored-by: Pierre Lehnen <55164754+pierre-lehnen-rc@users.noreply.github.com>

* Added few tests

Co-authored-by: Diego Sampaio <chinello@gmail.com>
Co-authored-by: Pierre Lehnen <55164754+pierre-lehnen-rc@users.noreply.github.com>
Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
pull/26985/head^2
Guilherme Gazzo 3 years ago committed by GitHub
parent 3715e0d109
commit 4afcbf64a2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 1
      apps/meteor/.mocharc.js
  2. 13
      apps/meteor/app/importer/server/classes/ImportDataConverter.ts
  3. 1
      apps/meteor/app/utils/client/index.ts
  4. 9
      apps/meteor/app/utils/lib/templateVarHandler.js
  5. 91
      apps/meteor/ee/server/lib/ldap/Manager.ts
  6. 209
      apps/meteor/ee/server/lib/ldap/copyCustomFieldsLDAP.spec.ts
  7. 71
      apps/meteor/ee/server/lib/ldap/copyCustomFieldsLDAP.ts
  8. 23
      apps/meteor/ee/server/lib/ldap/getNestedProp.spec.ts
  9. 7
      apps/meteor/ee/server/lib/ldap/getNestedProp.ts
  10. 87
      apps/meteor/ee/server/lib/ldap/replacesNestedValues.spec.ts
  11. 26
      apps/meteor/ee/server/lib/ldap/replacesNestedValues.ts

@ -24,6 +24,7 @@ module.exports = {
...base, // see https://github.com/mochajs/mocha/issues/3916
exit: true,
spec: [
'ee/server/lib/ldap/*.spec.ts',
'ee/tests/**/*.tests.ts',
'ee/tests/**/*.spec.ts',
'tests/unit/app/**/*.spec.ts',

@ -217,7 +217,10 @@ export class ImportDataConverter {
continue;
}
updateData.$set[keyPath] = source[key];
updateData.$set = {
...updateData.$set,
...{ [keyPath]: source[key] },
};
}
};
@ -237,16 +240,16 @@ export class ImportDataConverter {
}
// #ToDo: #TODO: Move this to the model class
const updateData: Record<string, any> = {
$set: {
const updateData: Record<string, any> = Object.assign(Object.create(null), {
$set: Object.assign(Object.create(null), {
...(userData.roles && { roles: userData.roles }),
...(userData.type && { type: userData.type }),
...(userData.statusText && { statusText: userData.statusText }),
...(userData.bio && { bio: userData.bio }),
...(userData.services?.ldap && { ldap: true }),
...(userData.avatarUrl && { _pendingAvatarUrl: userData.avatarUrl }),
},
};
}),
});
this.addCustomFields(updateData, userData);
this.addUserServices(updateData, userData);

@ -9,6 +9,5 @@ export { getUserNotificationPreference } from '../lib/getUserNotificationPrefere
export { getAvatarColor } from '../lib/getAvatarColor';
export { getURL } from '../lib/getURL';
export { placeholders } from '../lib/placeholders';
export { templateVarHandler } from '../lib/templateVarHandler';
export { APIClient } from './lib/RestApiClient';
export { secondsToHHMMSS } from '../../../lib/utils/secondsToHHMMSS';

@ -1,11 +1,6 @@
import { Meteor } from 'meteor/meteor';
import { Logger } from '../../../server/lib/logger/Logger';
let logger;
if (Meteor.isServer) {
const { Logger } = require('../../../server/lib/logger/Logger');
logger = new Logger('TemplateVarHandler');
}
const logger = new Logger('TemplateVarHandler');
export const templateVarHandler = function (variable, object) {
const templateRegex = /#{([\w\-]+)}/gi;

@ -1,4 +1,3 @@
import _ from 'underscore';
import type ldapjs from 'ldapjs';
import type { ILDAPEntry, IUser, IRoom, ICreatedRoom, IRole, IImportUser } from '@rocket.chat/core-typings';
import { Users as UsersRaw, Roles, Subscriptions as SubscriptionsRaw } from '@rocket.chat/models';
@ -10,11 +9,11 @@ import { LDAPDataConverter } from '../../../../server/lib/ldap/DataConverter';
import { LDAPConnection } from '../../../../server/lib/ldap/Connection';
import { LDAPManager } from '../../../../server/lib/ldap/Manager';
import { logger, searchLogger, mapLogger } from '../../../../server/lib/ldap/Logger';
import { templateVarHandler } from '../../../../app/utils/lib/templateVarHandler';
import { addUserToRoom, removeUserFromRoom, createRoom } from '../../../../app/lib/server/functions';
import { syncUserRoles } from '../syncUserRoles';
import { Team } from '../../../../server/sdk';
import { ensureArray } from '../../../../lib/utils/arrayUtils';
import { copyCustomFieldsLDAP } from './copyCustomFieldsLDAP';
export class LDAPEEManager extends LDAPManager {
public static async sync(): Promise<void> {
@ -506,76 +505,16 @@ export class LDAPEEManager extends LDAPManager {
}
public static copyCustomFields(ldapUser: ILDAPEntry, userData: IImportUser): void {
if (!settings.get<boolean>('LDAP_Sync_Custom_Fields')) {
return;
}
const customFieldsSettings = settings.get<string>('Accounts_CustomFields');
const customFieldsMap = settings.get<string>('LDAP_CustomFieldMap');
if (!customFieldsMap || !customFieldsSettings) {
if (customFieldsMap) {
logger.debug('Skipping LDAP custom fields because there are no custom fields configured.');
}
return;
}
let map: Record<string, string>;
try {
map = JSON.parse(customFieldsMap) as Record<string, string>;
} catch (error) {
logger.error('Failed to parse LDAP Custom Fields mapping');
logger.error(error);
return;
}
let customFields: Record<string, any>;
try {
customFields = JSON.parse(customFieldsSettings) as Record<string, any>;
} catch (error) {
logger.error('Failed to parse Custom Fields');
logger.error(error);
return;
}
_.map(map, (userField, ldapField) => {
if (!this.getCustomField(customFields, userField)) {
logger.debug(`User attribute does not exist: ${userField}`);
return;
}
if (!userData.customFields) {
userData.customFields = {};
}
const value = templateVarHandler(ldapField, ldapUser);
if (value) {
let ref: Record<string, any> = userData.customFields;
const attributeNames = userField.split('.');
let previousKey: string | undefined;
for (const key of attributeNames) {
if (previousKey) {
if (ref[previousKey] === undefined) {
ref[previousKey] = {};
} else if (typeof ref[previousKey] !== 'object') {
logger.error(`Failed to assign custom field: ${userField}`);
return;
}
ref = ref[previousKey];
}
previousKey = key;
}
if (previousKey) {
ref[previousKey] = value;
logger.debug(`user.customFields.${userField} changed to: ${value}`);
}
}
});
return copyCustomFieldsLDAP(
{
ldapUser,
userData,
customFieldsSettings: settings.get<string>('Accounts_CustomFields'),
customFieldsMap: settings.get<string>('LDAP_CustomFieldMap'),
syncCustomFields: settings.get<boolean>('LDAP_Sync_Custom_Fields'),
},
logger,
);
}
private static async importNewUsers(ldap: LDAPConnection, converter: LDAPDataConverter): Promise<void> {
@ -660,12 +599,4 @@ export class LDAPEEManager extends LDAPManager {
}
}
}
private static getCustomField(customFields: Record<string, any>, property: string): any {
try {
return _.reduce(property.split('.'), (acc, el) => acc[el], customFields);
} catch {
// ignore errors
}
}
}

@ -0,0 +1,209 @@
import type { IImportUser, ILDAPEntry } from '@rocket.chat/core-typings';
import { expect, spy } from 'chai';
import { copyCustomFieldsLDAP } from './copyCustomFieldsLDAP';
import type { Logger } from '../../../../app/logger/server';
describe('LDAP copyCustomFieldsLDAP', () => {
it('should copy custom fields from ldapUser to rcUser', () => {
const ldapUser = {
mail: 'test@test.com',
givenName: 'Test',
} as unknown as ILDAPEntry;
const userData = {
name: 'Test',
username: 'test',
} as unknown as IImportUser;
copyCustomFieldsLDAP(
{
ldapUser,
userData,
syncCustomFields: true,
customFieldsSettings: JSON.stringify({
mappedGivenName: { type: 'text', required: false },
}),
customFieldsMap: JSON.stringify({
givenName: 'mappedGivenName',
}),
},
{
debug: () => undefined,
error: () => undefined,
} as unknown as Logger,
);
expect(userData).to.have.property('customFields');
expect(userData.customFields).to.be.eql({ mappedGivenName: 'Test' });
});
it('should copy custom fields from ldapUser to rcUser already having other custom fields', () => {
const ldapUser = {
mail: 'test@test.com',
givenName: 'Test',
} as unknown as ILDAPEntry;
const userData = {
name: 'Test',
username: 'test',
customFields: {
custom: 'Test',
},
} as unknown as IImportUser;
copyCustomFieldsLDAP(
{
ldapUser,
userData,
syncCustomFields: true,
customFieldsSettings: JSON.stringify({
mappedGivenName: { type: 'text', required: false },
}),
customFieldsMap: JSON.stringify({
givenName: 'mappedGivenName',
}),
},
{
debug: () => undefined,
error: () => undefined,
} as unknown as Logger,
);
expect(userData).to.have.property('customFields');
expect(userData.customFields).to.be.eql({ custom: 'Test', mappedGivenName: 'Test' });
});
it('should not copy custom fields from ldapUser to rcUser if syncCustomFields is false', () => {
const ldapUser = {
mail: 'test@test.com',
givenName: 'Test',
} as unknown as ILDAPEntry;
const userData = {
name: 'Test',
username: 'test',
} as unknown as IImportUser;
copyCustomFieldsLDAP(
{
ldapUser,
userData,
syncCustomFields: false,
customFieldsSettings: JSON.stringify({
mappedGivenName: { type: 'text', required: false },
}),
customFieldsMap: JSON.stringify({
givenName: 'mappedGivenName',
}),
},
{
debug: () => undefined,
error: () => undefined,
} as unknown as Logger,
);
expect(userData).to.not.have.property('customFields');
});
it('should call logger.error if customFieldsSettings is not a valid JSON', () => {
const debug = spy();
const error = spy();
const ldapUser = {
mail: 'test@test.com',
givenName: 'Test',
} as unknown as ILDAPEntry;
const userData = {
name: 'Test',
username: 'test',
} as unknown as IImportUser;
copyCustomFieldsLDAP(
{
ldapUser,
userData,
syncCustomFields: true,
customFieldsSettings: `${JSON.stringify({
mappedGivenName: { type: 'text', required: false },
})}}`,
customFieldsMap: JSON.stringify({
givenName: 'mappedGivenName',
}),
},
{
debug,
error,
} as unknown as Logger,
);
expect(error).to.have.been.called.exactly(1);
});
it('should call logger.error if customFieldsMap is not a valid JSON', () => {
const debug = spy();
const error = spy();
const ldapUser = {
mail: 'test@test.com',
givenName: 'Test',
} as unknown as ILDAPEntry;
const userData = {
name: 'Test',
username: 'test',
} as unknown as IImportUser;
copyCustomFieldsLDAP(
{
ldapUser,
userData,
syncCustomFields: true,
customFieldsSettings: JSON.stringify({
mappedGivenName: { type: 'text', required: false },
}),
customFieldsMap: `${JSON.stringify({
givenName: 'mappedGivenName',
})}}`,
},
{
debug,
error,
} as unknown as Logger,
);
expect(error).to.have.been.called.exactly(1);
});
it('should call logger.debug if some custom fields are not mapped but still mapping the other fields', () => {
const debug = spy();
const error = spy();
const ldapUser = {
mail: 'test@test.com',
givenName: 'Test',
} as unknown as ILDAPEntry;
const userData = {
name: 'Test',
username: 'test',
} as unknown as IImportUser;
copyCustomFieldsLDAP(
{
ldapUser,
userData,
syncCustomFields: true,
customFieldsSettings: JSON.stringify({
mappedGivenName: { type: 'text', required: false },
}),
customFieldsMap: JSON.stringify({
givenName: 'mappedGivenName',
test: 'test',
}),
},
{
debug,
error,
} as unknown as Logger,
);
expect(debug).to.have.been.called.exactly(1);
expect(userData).to.have.property('customFields');
expect(userData.customFields).to.be.eql({ mappedGivenName: 'Test' });
});
});

@ -0,0 +1,71 @@
import type { IImportUser, ILDAPEntry } from '@rocket.chat/core-typings';
import type { Logger } from '../../../../app/logger/server';
import { templateVarHandler } from '../../../../app/utils/lib/templateVarHandler';
import { getNestedProp } from './getNestedProp';
import { replacesNestedValues } from './replacesNestedValues';
export const copyCustomFieldsLDAP = (
{
ldapUser,
userData,
customFieldsSettings,
customFieldsMap,
syncCustomFields,
}: {
ldapUser: ILDAPEntry;
userData: IImportUser;
syncCustomFields: boolean;
customFieldsSettings: string;
customFieldsMap: string;
},
logger: Logger,
): void => {
if (!syncCustomFields) {
return;
}
if (!customFieldsMap || !customFieldsSettings) {
if (customFieldsMap) {
logger.debug('Skipping LDAP custom fields because there are no custom map fields configured.');
return;
}
logger.debug('Skipping LDAP custom fields because there are no custom fields configured.');
return;
}
const map: Record<string, string> = (() => {
try {
return JSON.parse(customFieldsMap);
} catch (err) {
logger.error({ msg: 'Error parsing LDAP custom fields map.', err });
}
})();
if (!map) {
return;
}
let customFields: Record<string, any>;
try {
customFields = JSON.parse(customFieldsSettings) as Record<string, any>;
} catch (err) {
logger.error({ msg: 'Failed to parse Custom Fields', err });
return;
}
Object.entries(map).forEach(([ldapField, userField]) => {
if (!getNestedProp(customFields, userField)) {
logger.debug(`User attribute does not exist: ${userField}`);
return;
}
const value = templateVarHandler(ldapField, ldapUser);
if (!value) {
return;
}
userData.customFields = replacesNestedValues({ ...userData.customFields }, userField, value);
});
};

@ -0,0 +1,23 @@
import { expect } from 'chai';
import { getNestedProp } from './getNestedProp';
describe('LDAP getNestedProp', () => {
it('should find shallow values', () => {
const customFields = {
foo: 'bar',
};
expect(getNestedProp(customFields, 'foo')).to.equal('bar');
});
it('should find deep values', () => {
const customFields = {
foo: {
bar: 'baz',
},
};
expect(getNestedProp(customFields, 'foo.bar')).to.equal('baz');
});
});

@ -0,0 +1,7 @@
export const getNestedProp = (customFields: Record<string, any>, property: string): unknown => {
try {
return property.split('.').reduce((acc, el) => acc[el], customFields);
} catch {
// ignore errors
}
};

@ -0,0 +1,87 @@
import { expect } from 'chai';
import { replacesNestedValues } from './replacesNestedValues';
describe('LDAP replacesNestedValues', () => {
it('should replace shallow values', () => {
const result = replacesNestedValues(
{
a: 1,
},
'a',
2,
);
expect(result).to.eql({
a: 2,
});
});
it('should replace undefined values', () => {
const result = replacesNestedValues({}, 'a', 2);
expect(result).to.eql({
a: 2,
});
});
it('should replace nested values', () => {
const result = replacesNestedValues(
{
a: {
b: 1,
},
},
'a.b',
2,
);
expect(result).to.eql({
a: {
b: 2,
},
});
});
it('should replace undefined nested values', () => {
const result = replacesNestedValues(
{
a: {},
},
'a.b',
2,
);
expect(result).to.eql({
a: {
b: 2,
},
});
});
it('should fail if the value being replaced is not an object', () => {
expect(() =>
replacesNestedValues(
{
a: [],
},
'a.b',
2,
),
).to.throw();
expect(() =>
replacesNestedValues(
{
a: 1,
},
'a.b',
2,
),
).to.throw();
expect(() =>
replacesNestedValues(
{
a: { b: [] },
},
'a.b',
2,
),
).to.throw();
});
});

@ -0,0 +1,26 @@
export const replacesNestedValues = (obj: Record<string, unknown>, key: string, value: unknown): Record<string, unknown> => {
const keys = key.split('.');
const lastKey = keys.shift();
if (!lastKey) {
throw new Error(`Failed to assign custom field: ${key}`);
}
if (keys.length && obj[lastKey] !== undefined && (typeof obj[lastKey] !== 'object' || Array.isArray(obj[lastKey]))) {
throw new Error(`Failed to assign custom field: ${key}`);
}
if (keys.length === 0 && typeof obj[lastKey] === 'object') {
throw new Error(`Failed to assign custom field: ${key}`);
}
return {
...obj,
...(keys.length === 0 && {
[lastKey]: value,
}),
...(keys.length > 0 && {
[lastKey]: replacesNestedValues(obj[lastKey] as Record<string, unknown>, keys.join('.'), value),
}),
};
};
Loading…
Cancel
Save