Skip to content

Commit e06b0cd

Browse files
ggazzoggazzo
authored
Merge branch 'develop' into release-8.0.0
2 parents 7596652 + 5836726 commit e06b0cd

File tree

12 files changed

+341
-3
lines changed

12 files changed

+341
-3
lines changed

‎.changeset/rotten-pugs-trade.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@rocket.chat/meteor": patch
3+
"@rocket.chat/tools": patch
4+
---
5+
6+
Adds improvements to the push notifications logic; the logic now truncates messages and titles larger than 240, and 65 characters respectively.

‎.github/actions/setup-node/action.yml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,14 @@ runs:
6969
with:
7070
deno-version: ${{ inputs.deno-version }}
7171

72+
- name: Configure mongodb-memory-server cache
73+
shell: bash
74+
run: |
75+
CACHE_DIR="${GITHUB_WORKSPACE}/node_modules/.cache/mongodb-memory-server"
76+
mkdir -p "$CACHE_DIR"
77+
echo "MONGOMS_DOWNLOAD_DIR=$CACHE_DIR" >> $GITHUB_ENV
78+
echo "MONGOMS_PREFER_GLOBAL_PATH=false" >> $GITHUB_ENV
79+
7280
- name: yarn login
7381
shell: bash
7482
if: inputs.NPM_TOKEN

‎apps/meteor/app/push/server/push.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { IAppsTokens, RequiredField, Optional, IPushNotificationConfig } from '@rocket.chat/core-typings';
22
import { AppsTokens } from '@rocket.chat/models';
33
import { serverFetch as fetch } from '@rocket.chat/server-fetch';
4-
import { pick } from '@rocket.chat/tools';
4+
import { pick, truncateString } from '@rocket.chat/tools';
55
import Ajv from 'ajv';
66
import { JWT } from 'google-auth-library';
77
import { Match, check } from 'meteor/check';
@@ -15,6 +15,9 @@ import { settings } from '../../settings/server';
1515

1616
export const _matchToken = Match.OneOf({ apn: String }, { gcm: String });
1717

18+
const PUSH_TITLE_LIMIT = 65;
19+
const PUSH_MESSAGE_BODY_LIMIT = 240;
20+
1821
const ajv = new Ajv({
1922
coerceTypes: true,
2023
});
@@ -459,8 +462,10 @@ class PushClass {
459462
createdBy: '<SERVER>',
460463
sent: false,
461464
sending: 0,
465+
title: truncateString(options.title, PUSH_TITLE_LIMIT),
466+
text: truncateString(options.text, PUSH_MESSAGE_BODY_LIMIT),
462467

463-
...pick(options, 'from', 'title', 'text', 'userId', 'payload', 'badge', 'sound', 'notId', 'priority'),
468+
...pick(options, 'from', 'userId', 'payload', 'badge', 'sound', 'notId', 'priority'),
464469

465470
...(this.hasApnOptions(options)
466471
? {

‎apps/meteor/ee/server/lib/audit/methods.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,12 @@ Meteor.methods<ServerMethods>({
163163
if (type === 'u') {
164164
const usersId = await getUsersIdFromUserName(usernames);
165165
query['u._id'] = { $in: usersId };
166+
167+
const abacRooms = await Rooms.findAllPrivateRoomsWithAbacAttributes({ projection: { _id: 1 } })
168+
.map((doc) => doc._id)
169+
.toArray();
170+
171+
query.rid = { $nin: abacRooms };
166172
} else {
167173
const roomInfo = await getRoomInfoByAuditParams({ type, roomId: rid, users: usernames, visitor, agent, userId: user._id });
168174
if (!roomInfo) {

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

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,167 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
416416
});
417417
});
418418

419+
describe('Audit messages by user and ABAC-managed rooms', () => {
420+
let auditUser: IUser;
421+
let auditUserCreds: Credentials;
422+
let auditRoom: IRoom;
423+
const auditAttrKey = `audit_attr_${Date.now()}`;
424+
let startDate: Date;
425+
let endDate: Date;
426+
427+
before(async () => {
428+
startDate = new Date();
429+
endDate = new Date(startDate.getTime() + 1000 * 60);
430+
431+
auditUser = await createUser();
432+
auditUserCreds = await login(auditUser.username, password);
433+
434+
await request
435+
.post(`${v1}/abac/attributes`)
436+
.set(credentials)
437+
.send({ key: auditAttrKey, values: ['v1'] })
438+
.expect(200);
439+
440+
await addAbacAttributesToUserDirectly(auditUser._id, [{ key: auditAttrKey, values: ['v1'] }]);
441+
await addAbacAttributesToUserDirectly(credentials['X-User-Id'], [{ key: auditAttrKey, values: ['v1'] }]);
442+
443+
const roomRes = await createRoom({ type: 'p', name: `abac-audit-user-room-${Date.now()}`, members: [auditUser.username!] });
444+
auditRoom = roomRes.body.group as IRoom;
445+
446+
await request
447+
.post(`${v1}/abac/rooms/${auditRoom._id}/attributes/${auditAttrKey}`)
448+
.set(credentials)
449+
.send({ values: ['v1'] })
450+
.expect(200);
451+
});
452+
453+
after(async () => {
454+
await deleteRoom({ type: 'p', roomId: auditRoom._id });
455+
await deleteUser(auditUser);
456+
});
457+
458+
it("should return no messages when auditing a user that's part of an ABAC-managed room", async () => {
459+
await request
460+
.post(`${v1}/chat.sendMessage`)
461+
.set(auditUserCreds)
462+
.send({ message: { rid: auditRoom._id, msg: 'audit message in abac room' } })
463+
.expect(200);
464+
465+
await request
466+
.post(methodCall('auditGetMessages'))
467+
.set(credentials)
468+
.send({
469+
message: JSON.stringify({
470+
method: 'auditGetMessages',
471+
params: [
472+
{
473+
type: 'u',
474+
msg: 'audit message in abac room',
475+
startDate: { $date: startDate },
476+
endDate: { $date: endDate },
477+
rid: '',
478+
users: [auditUser.username],
479+
visitor: '',
480+
agent: '',
481+
},
482+
],
483+
id: 'abac-audit-1',
484+
msg: 'method',
485+
}),
486+
})
487+
.expect(200)
488+
.expect((res) => {
489+
const parsed = JSON.parse(res.body.message);
490+
expect(parsed).to.have.property('result');
491+
expect(parsed.result).to.be.an('array').that.is.empty;
492+
});
493+
});
494+
495+
it('should return no messages when auditing a user that WAS part of an ABAC-managed room', async () => {
496+
await request
497+
.post(`${v1}/chat.sendMessage`)
498+
.set(auditUserCreds)
499+
.send({ message: { rid: auditRoom._id, msg: 'audit message before removal' } })
500+
.expect(200);
501+
502+
await request.post(`${v1}/groups.kick`).set(credentials).send({ roomId: auditRoom._id, username: auditUser.username }).expect(200);
503+
504+
await request
505+
.post(methodCall('auditGetMessages'))
506+
.set(credentials)
507+
.send({
508+
message: JSON.stringify({
509+
method: 'auditGetMessages',
510+
params: [
511+
{
512+
type: 'u',
513+
msg: 'audit message before removal',
514+
startDate: { $date: startDate },
515+
endDate: { $date: endDate },
516+
rid: '',
517+
users: [auditUser.username],
518+
visitor: '',
519+
agent: '',
520+
},
521+
],
522+
id: 'abac-audit-2',
523+
msg: 'method',
524+
}),
525+
})
526+
.expect(200)
527+
.expect((res) => {
528+
const parsed = JSON.parse(res.body.message);
529+
expect(parsed).to.have.property('result');
530+
expect(parsed.result).to.be.an('array').that.is.empty;
531+
});
532+
});
533+
534+
it("should return messages when auditing a user that is part of a room that's no longer ABAC-managed", async () => {
535+
await request
536+
.post(`${v1}/groups.invite`)
537+
.set(credentials)
538+
.send({ roomId: auditRoom._id, usernames: [auditUser.username] })
539+
.expect(200);
540+
541+
await request.delete(`${v1}/abac/rooms/${auditRoom._id}/attributes/${auditAttrKey}`).set(credentials).expect(200);
542+
543+
await request
544+
.post(`${v1}/chat.sendMessage`)
545+
.set(auditUserCreds)
546+
.send({ message: { rid: auditRoom._id, msg: 'audit message after room no longer abac' } })
547+
.expect(200);
548+
549+
await request
550+
.post(methodCall('auditGetMessages'))
551+
.set(credentials)
552+
.send({
553+
message: JSON.stringify({
554+
method: 'auditGetMessages',
555+
params: [
556+
{
557+
type: 'u',
558+
msg: 'audit message after room no longer abac',
559+
startDate: { $date: startDate },
560+
endDate: { $date: endDate },
561+
rid: '',
562+
users: [auditUser.username],
563+
visitor: '',
564+
agent: '',
565+
},
566+
],
567+
id: 'abac-audit-3',
568+
msg: 'method',
569+
}),
570+
})
571+
.expect(200)
572+
.expect((res) => {
573+
const parsed = JSON.parse(res.body.message);
574+
expect(parsed).to.have.property('result');
575+
expect(parsed.result).to.be.an('array').with.lengthOf.greaterThan(0);
576+
});
577+
});
578+
});
579+
419580
it('PUT room attribute should replace values and keep inUse=true', async () => {
420581
await request
421582
.put(`${v1}/abac/rooms/${testRoom._id}/attributes/${updatedKey}`)

‎apps/meteor/tests/end-to-end/api/audit.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ import { IS_EE } from '../../e2e/config/constants';
136136

137137
expect(message.result).to.be.an('array').with.lengthOf.greaterThan(1);
138138
const entry = message.result.find((audition: any) => {
139-
return audition.fields.rids.includes(testChannel._id);
139+
return audition.fields?.rids?.includes(testChannel._id);
140140
});
141141
expect(entry).to.have.property('u').that.is.an('object').deep.equal({
142142
_id: 'rocketchat.internal.admin.test',

‎apps/meteor/tests/unit/app/push/push.spec.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import type { IPushNotificationConfig } from '@rocket.chat/core-typings/src/IPushNotificationConfig';
2+
import { pick, truncateString } from '@rocket.chat/tools';
3+
import { expect } from 'chai';
4+
import proxyquire from 'proxyquire';
5+
import sinon from 'sinon';
6+
7+
const loggerStub = { debug: sinon.stub(), warn: sinon.stub(), error: sinon.stub(), info: sinon.stub(), log: sinon.stub() };
8+
const settingsStub = { get: sinon.stub().returns('') };
9+
10+
const { Push } = proxyquire.noCallThru().load('../../../../app/push/server/push', {
11+
'./logger': { logger: loggerStub },
12+
'../../settings/server': { settings: settingsStub },
13+
'@rocket.chat/tools': { pick, truncateString },
14+
'meteor/check': {
15+
check: sinon.stub(),
16+
Match: {
17+
Optional: () => sinon.stub(),
18+
Integer: Number,
19+
OneOf: () => sinon.stub(),
20+
test: sinon.stub().returns(true),
21+
},
22+
},
23+
'meteor/meteor': {
24+
Meteor: {
25+
absoluteUrl: sinon.stub().returns('http://localhost'),
26+
},
27+
},
28+
});
29+
30+
describe('Push Notifications [PushClass]', () => {
31+
afterEach(() => {
32+
sinon.restore();
33+
});
34+
35+
describe('send()', () => {
36+
let sendNotificationStub: sinon.SinonStub;
37+
beforeEach(() => {
38+
sendNotificationStub = sinon.stub(Push, 'sendNotification').resolves({ apn: [], gcm: [] });
39+
});
40+
41+
it('should call sendNotification with required fields', async () => {
42+
const options: IPushNotificationConfig = {
43+
from: 'test',
44+
title: 'title',
45+
text: 'body',
46+
userId: 'user1',
47+
apn: { category: 'MESSAGE' },
48+
gcm: { style: 'inbox', image: 'url' },
49+
};
50+
51+
await Push.send(options);
52+
53+
expect(sendNotificationStub.calledOnce).to.be.true;
54+
55+
const notification = sendNotificationStub.firstCall.args[0];
56+
expect(notification.from).to.equal('test');
57+
expect(notification.title).to.equal('title');
58+
expect(notification.text).to.equal('body');
59+
expect(notification.userId).to.equal('user1');
60+
});
61+
62+
it('should truncate text if longer than 240 chars', async () => {
63+
const longText = 'a'.repeat(300);
64+
const options: IPushNotificationConfig = {
65+
from: 'test',
66+
title: 'title',
67+
text: longText,
68+
userId: 'user1',
69+
apn: { category: 'MESSAGE' },
70+
gcm: { style: 'inbox', image: 'url' },
71+
};
72+
73+
await Push.send(options);
74+
75+
const notification = sendNotificationStub.firstCall.args[0];
76+
77+
expect(notification.text.length).to.equal(240);
78+
});
79+
80+
it('should truncate title if longer than 65 chars', async () => {
81+
const longTitle = 'a'.repeat(100);
82+
const options: IPushNotificationConfig = {
83+
from: 'test',
84+
title: longTitle,
85+
text: 'bpdu',
86+
userId: 'user1',
87+
apn: { category: 'MESSAGE' },
88+
gcm: { style: 'inbox', image: 'url' },
89+
};
90+
91+
await Push.send(options);
92+
93+
const notification = sendNotificationStub.firstCall.args[0];
94+
95+
expect(notification.title.length).to.equal(65);
96+
});
97+
98+
it('should throw if userId is missing', async () => {
99+
const options = {
100+
from: 'test',
101+
title: 'title',
102+
text: 'body',
103+
apn: { category: 'MESSAGE' },
104+
gcm: { style: 'inbox', image: 'url' },
105+
};
106+
107+
await expect(Push.send(options)).to.be.rejectedWith('No userId found');
108+
109+
expect(sendNotificationStub.called).to.be.false;
110+
});
111+
});
112+
});

‎ee/packages/abac/package.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@
1717
"testunit": "jest",
1818
"typecheck": "tsc --noEmit --skipLibCheck"
1919
},
20+
"config": {
21+
"mongodbMemoryServer": {
22+
"downloadDir": "../../../node_modules/.cache/mongodb-memory-server",
23+
"preferGlobalPath": false
24+
}
25+
},
2026
"volta": {
2127
"extends": "../../../package.json"
2228
},

‎packages/model-typings/src/models/IRoomsModel.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ export interface IRoomsModel extends IBaseModel<IRoom> {
232232
findByType(type: IRoom['t'], options?: FindOptions<IRoom>): FindCursor<IRoom>;
233233
findByTypeInIds(type: IRoom['t'], ids: string[], options?: FindOptions<IRoom>): FindCursor<IRoom>;
234234
findPrivateRoomsByIdsWithAbacAttributes(ids: string[], options?: FindOptions<IRoom>): FindCursor<IRoom>;
235+
findAllPrivateRoomsWithAbacAttributes(options?: FindOptions<IRoom>): FindCursor<IRoom>;
235236
findBySubscriptionUserId(userId: string, options?: FindOptions<IRoom>): Promise<FindCursor<IRoom>>;
236237
findBySubscriptionUserIdUpdatedAfter(userId: string, updatedAfter: Date, options?: FindOptions<IRoom>): Promise<FindCursor<IRoom>>;
237238
findByNameAndTypeNotDefault(

‎packages/models/src/models/Rooms.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,6 +1278,15 @@ export class RoomsRaw extends BaseRaw<IRoom> implements IRoomsModel {
12781278
return this.find(query, options);
12791279
}
12801280

1281+
findAllPrivateRoomsWithAbacAttributes(options: FindOptions<IRoom> = {}): FindCursor<IRoom> {
1282+
const query: Filter<IRoom> = {
1283+
t: 'p',
1284+
abacAttributes: { $exists: true, $ne: [] },
1285+
};
1286+
1287+
return this.find(query, options);
1288+
}
1289+
12811290
async findBySubscriptionUserId(userId: IUser['_id'], options: FindOptions<IRoom> = {}): Promise<FindCursor<IRoom>> {
12821291
const data = (await Subscriptions.findByUserId(userId, { projection: { rid: 1 } }).toArray()).map((item) => item.rid);
12831292

0 commit comments

Comments
 (0)