diff --git a/.changeset/silly-clouds-hear.md b/.changeset/silly-clouds-hear.md new file mode 100644 index 0000000000000..2b15b7fa4ed06 --- /dev/null +++ b/.changeset/silly-clouds-hear.md @@ -0,0 +1,6 @@ +--- +"@rocket.chat/rest-typings": minor +"@rocket.chat/meteor": minor +--- + +Adds `customFields` and `includeCustomFields` parameters to the `users.list` endpoint, filtering users by exact custom field values and returning the fields the caller may read diff --git a/apps/meteor/server/api/lib/parseCustomFieldsFilter.spec.ts b/apps/meteor/server/api/lib/parseCustomFieldsFilter.spec.ts new file mode 100644 index 0000000000000..3fdd971b0b88c --- /dev/null +++ b/apps/meteor/server/api/lib/parseCustomFieldsFilter.spec.ts @@ -0,0 +1,45 @@ +import { expect } from 'chai'; +import { describe, it } from 'mocha'; + +import { parseCustomFieldsFilter } from './parseCustomFieldsFilter'; + +describe('parseCustomFieldsFilter', () => { + it('should map a key to an exact-match filter', () => { + expect(parseCustomFieldsFilter('{"CustomerID":"acct-4821"}')).to.deep.equal({ + 'customFields.CustomerID': 'acct-4821', + }); + }); + + it('should combine multiple keys', () => { + expect(parseCustomFieldsFilter('{"CustomerID":"1","BAID":"2"}')).to.deep.equal({ + 'customFields.CustomerID': '1', + 'customFields.BAID': '2', + }); + }); + + it('should reject malformed JSON as a shape error, not an empty payload', () => { + expect(() => parseCustomFieldsFilter('{ssn:')).to.throw('customFields must be a JSON object'); + }); + + it('should reject a payload that is not a plain object', () => { + expect(() => parseCustomFieldsFilter('["a"]')).to.throw('must be a JSON object'); + expect(() => parseCustomFieldsFilter('"a"')).to.throw('must be a JSON object'); + expect(() => parseCustomFieldsFilter('null')).to.throw('must be a JSON object'); + }); + + it('should reject an empty object', () => { + expect(() => parseCustomFieldsFilter('{}')).to.throw('at least one field'); + }); + + it('should reject values that are not non-empty strings', () => { + expect(() => parseCustomFieldsFilter('{"a":{"$ne":null}}')).to.throw('non-empty string'); + expect(() => parseCustomFieldsFilter('{"a":123}')).to.throw('non-empty string'); + expect(() => parseCustomFieldsFilter('{"a":["x"]}')).to.throw('non-empty string'); + expect(() => parseCustomFieldsFilter('{"a":""}')).to.throw('non-empty string'); + }); + + it('should reject keys carrying an operator or a nested path', () => { + expect(() => parseCustomFieldsFilter('{"$where":"return true"}')).to.throw('not allowed'); + expect(() => parseCustomFieldsFilter('{"a.b":"x"}')).to.throw('not allowed'); + }); +}); diff --git a/apps/meteor/server/api/lib/parseCustomFieldsFilter.ts b/apps/meteor/server/api/lib/parseCustomFieldsFilter.ts new file mode 100644 index 0000000000000..c29e93d6833ac --- /dev/null +++ b/apps/meteor/server/api/lib/parseCustomFieldsFilter.ts @@ -0,0 +1,29 @@ +import { isRecord, wrapExceptions } from '@rocket.chat/tools'; + +export function parseCustomFieldsFilter(raw: string): Record { + const parsed = wrapExceptions(() => JSON.parse(raw)).suppress(); + + if (!isRecord(parsed)) { + throw new Error('customFields must be a JSON object'); + } + + const entries = Object.entries(parsed); + + if (!entries.length) { + throw new Error('customFields must declare at least one field'); + } + + return Object.fromEntries( + entries.map(([key, value]) => { + if (typeof value !== 'string' || !value) { + throw new Error(`customFields.${key} must be a non-empty string`); + } + + if (key.includes('$') || key.includes('.')) { + throw new Error(`The given key contains a period or an operator, which is not allowed. Key: ${key}`); + } + + return [`customFields.${key}`, value]; + }), + ); +} diff --git a/apps/meteor/server/api/v1/users.ts b/apps/meteor/server/api/v1/users.ts index dbdd9f188c916..29eba1d917673 100644 --- a/apps/meteor/server/api/v1/users.ts +++ b/apps/meteor/server/api/v1/users.ts @@ -89,6 +89,7 @@ import { getUserFromParams } from '../lib/getUserFromParams'; import { getUserInfo } from '../lib/getUserInfo'; import { isUserFromParams } from '../lib/isUserFromParams'; import { isValidQuery } from '../lib/isValidQuery'; +import { parseCustomFieldsFilter } from '../lib/parseCustomFieldsFilter'; import { queryFiltersStatus } from '../lib/queryFiltersStatus'; import { findPaginatedUsersByStatus, findUsersToAutocomplete, getInclusiveFields, getNonEmptyFields, getNonEmptyQuery } from '../lib/users'; @@ -676,6 +677,10 @@ API.v1.addRoute( } const canViewFullOtherUserInfo = await hasPermissionAsync(this.user, 'view-full-other-user-info'); + if ((this.queryParams.customFields || this.queryParams.includeCustomFields === 'true') && !canViewFullOtherUserInfo) { + return API.v1.forbidden(); + } + const { offset, count } = await getPaginationItems(this.queryParams); const { sort, fields, query } = await this.parseJsonQuery(); @@ -719,6 +724,10 @@ API.v1.addRoute( throw new Meteor.Error('error-invalid-query', isValidQuery.errors.join('\n')); } + if ('customFields' in this.queryParams && this.queryParams.customFields) { + Object.assign(nonEmptyQuery, parseCustomFieldsFilter(this.queryParams.customFields)); + } + const hidden = await getUsersHiddenFrom(this.userId); if (hidden && queryFiltersStatus(query)) { @@ -750,7 +759,10 @@ API.v1.addRoute( $match: nonEmptyQuery, }, { - $project: inclusiveFields, + $project: { + ...inclusiveFields, + ...(this.queryParams.includeCustomFields === 'true' && { customFields: 1 }), + }, }, { $addFields: { diff --git a/apps/meteor/tests/end-to-end/api/users.ts b/apps/meteor/tests/end-to-end/api/users.ts index 19f6ed0687e9c..10a97c7d68552 100644 --- a/apps/meteor/tests/end-to-end/api/users.ts +++ b/apps/meteor/tests/end-to-end/api/users.ts @@ -1983,6 +1983,109 @@ describe('[Users]', () => { .end(done); }); + describe('custom fields filter', () => { + let cfUser: TestUser; + const cfValue = `ext-${Date.now()}`; + + before(async () => { + await updateSetting('Accounts_CustomFields', JSON.stringify({ externalId: { type: 'text', required: false } })); + + cfUser = await createUser(); + + await request + .post(api('users.update')) + .set(credentials) + .send({ userId: cfUser._id, data: { customFields: { externalId: cfValue } } }) + .expect(200); + }); + + after(async () => { + await Promise.all([ + updateSetting('Accounts_CustomFields', ''), + restorePermissionToRoles('view-full-other-user-info'), + deleteUser(cfUser), + ]); + }); + + it('should return only the user matching the custom field value', async () => { + const response = await request + .get(api('users.list')) + .set(credentials) + .query({ customFields: JSON.stringify({ externalId: cfValue }) }) + .expect('Content-Type', 'application/json') + .expect(200); + + expect(response.body).to.have.property('total', 1); + expect(response.body.users).to.be.an('array').with.lengthOf(1); + expect(response.body.users[0]).to.have.property('_id', cfUser._id); + }); + + it('should return nothing when the value does not match', async () => { + const response = await request + .get(api('users.list')) + .set(credentials) + .query({ customFields: JSON.stringify({ externalId: 'no-such-value' }) }) + .expect('Content-Type', 'application/json') + .expect(200); + + expect(response.body).to.have.property('total', 0); + }); + + it('should not accept a mongo operator as a value', async () => { + const response = await request + .get(api('users.list')) + .set(credentials) + .query({ customFields: JSON.stringify({ externalId: { $ne: null } }) }) + .expect('Content-Type', 'application/json') + .expect(400); + + expect(response.body).to.have.property('success', false); + }); + + it('should return the custom fields when includeCustomFields is set', async () => { + const response = await request + .get(api('users.list')) + .set(credentials) + .query({ customFields: JSON.stringify({ externalId: cfValue }), includeCustomFields: 'true' }) + .expect('Content-Type', 'application/json') + .expect(200); + + expect(response.body.users[0]).to.have.nested.property('customFields.externalId', cfValue); + }); + + it('should not return the custom fields unless includeCustomFields is set', async () => { + const response = await request + .get(api('users.list')) + .set(credentials) + .query({ customFields: JSON.stringify({ externalId: cfValue }) }) + .expect('Content-Type', 'application/json') + .expect(200); + + expect(response.body.users[0]).to.not.have.property('customFields'); + }); + + it('should reject an empty customFields parameter instead of ignoring it', async () => { + await request + .get(api('users.list')) + .set(credentials) + .query({ customFields: '' }) + .expect('Content-Type', 'application/json') + .expect(400); + }); + + it('should forbid both filtering and reading without view-full-other-user-info', async () => { + await updatePermission('view-full-other-user-info', ['admin']); + + await request + .get(api('users.list')) + .set(user2Credentials) + .query({ customFields: JSON.stringify({ externalId: cfValue }) }) + .expect(403); + + await request.get(api('users.list')).set(user2Credentials).query({ includeCustomFields: 'true' }).expect(403); + }); + }); + it('should query all users in the system when logged as normal user and `view-outside-room` not granted', async () => { await updatePermission('view-outside-room', ['admin']); await request diff --git a/packages/rest-typings/src/v1/users.ts b/packages/rest-typings/src/v1/users.ts index 1bd9a7c559306..0b6d23caa1ee2 100644 --- a/packages/rest-typings/src/v1/users.ts +++ b/packages/rest-typings/src/v1/users.ts @@ -156,7 +156,7 @@ export type UsersEndpoints = { '/v1/users.list': { GET: (params: UsersListParamsGET) => PaginatedResult<{ - users: DefaultUserInfo[]; + users: (DefaultUserInfo & Pick, 'customFields'>)[]; }>; }; diff --git a/packages/rest-typings/src/v1/users/UsersListParamsGET.ts b/packages/rest-typings/src/v1/users/UsersListParamsGET.ts index ae26ca8f66fc2..01bb45ed51de6 100644 --- a/packages/rest-typings/src/v1/users/UsersListParamsGET.ts +++ b/packages/rest-typings/src/v1/users/UsersListParamsGET.ts @@ -5,6 +5,8 @@ export type UsersListParamsGET = PaginatedRequest<{ fields?: string; query?: string; email?: string; + customFields?: string; + includeCustomFields?: 'true' | 'false'; }>; const UsersListParamsGetSchema = { @@ -16,6 +18,8 @@ const UsersListParamsGetSchema = { offset: { type: 'number', nullable: true }, sort: { type: 'string', nullable: true }, email: { type: 'string', minLength: 1, nullable: true }, + customFields: { type: 'string', minLength: 1, nullable: true }, + includeCustomFields: { type: 'string', enum: ['true', 'false'], nullable: true }, }, additionalProperties: false, };