From f1977e0f5710e377bba49501fe6876aee90d975d Mon Sep 17 00:00:00 2001 From: Tiago Evangelista Date: Wed, 12 Feb 2025 18:14:34 -0300 Subject: [PATCH 1/8] fix --- .../server/modules/notifications/notifications.module.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/meteor/server/modules/notifications/notifications.module.ts b/apps/meteor/server/modules/notifications/notifications.module.ts index a0394591aadf2..07a06d6e38c7f 100644 --- a/apps/meteor/server/modules/notifications/notifications.module.ts +++ b/apps/meteor/server/modules/notifications/notifications.module.ts @@ -105,7 +105,8 @@ export class NotificationsModule { if (!canAccess) { // verify if can preview messages from public channels if (room.t === 'c' && this.userId) { - return Authorization.hasPermission(this.userId, 'preview-c-room'); + const subscription = await Subscriptions.findOne({ rid: room._id }); + return !!subscription && Authorization.hasPermission(this.userId, 'preview-c-room'); } return false; } @@ -186,6 +187,7 @@ export class NotificationsModule { if (!this.userId) { return false; } + const canAccess = await Authorization.canAccessRoomId(room._id, this.userId); return canAccess; From 7cda41714b612fb87aa136e16b9fe4f039cd85a7 Mon Sep 17 00:00:00 2001 From: Tiago Evangelista Pinto Date: Thu, 13 Feb 2025 14:45:23 -0300 Subject: [PATCH 2/8] Create lemon-lions-learn.md --- .changeset/lemon-lions-learn.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/lemon-lions-learn.md diff --git a/.changeset/lemon-lions-learn.md b/.changeset/lemon-lions-learn.md new file mode 100644 index 0000000000000..8ed1d10bccf0a --- /dev/null +++ b/.changeset/lemon-lions-learn.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": patch +--- + +fixes the possibility of see new messages without being subscribed to a public channel. From 79c6ad352cf9320423db942878dcdc0cc71f63b3 Mon Sep 17 00:00:00 2001 From: Tiago Evangelista Date: Thu, 13 Feb 2025 15:57:58 -0300 Subject: [PATCH 3/8] review --- .../notifications/notifications.module.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/apps/meteor/server/modules/notifications/notifications.module.ts b/apps/meteor/server/modules/notifications/notifications.module.ts index 07a06d6e38c7f..27fd181e9a28f 100644 --- a/apps/meteor/server/modules/notifications/notifications.module.ts +++ b/apps/meteor/server/modules/notifications/notifications.module.ts @@ -102,15 +102,27 @@ export class NotificationsModule { } const canAccess = await Authorization.canAccessRoom(room, { _id: this.userId || '' }, extraData); + if (!canAccess) { // verify if can preview messages from public channels if (room.t === 'c' && this.userId) { - const subscription = await Subscriptions.findOne({ rid: room._id }); - return !!subscription && Authorization.hasPermission(this.userId, 'preview-c-room'); + return Authorization.hasPermission(this.userId, 'preview-c-room'); } return false; } + if (room.t === 'c' && this.userId) { + const hasPreviewPermission = await Authorization.hasPermission(this.userId, 'preview-c-room'); + + if (hasPreviewPermission) { + const subscription = await Subscriptions.findOneByRoomIdAndUserId(room._id, this.userId); + + return !!subscription; + } + + return false; + } + return true; }); @@ -187,7 +199,6 @@ export class NotificationsModule { if (!this.userId) { return false; } - const canAccess = await Authorization.canAccessRoomId(room._id, this.userId); return canAccess; From ba23d4c9f0b5f4c8e3de5f0518433604ede56fc2 Mon Sep 17 00:00:00 2001 From: Tiago Evangelista Pinto Date: Thu, 13 Feb 2025 16:20:57 -0300 Subject: [PATCH 4/8] Update apps/meteor/server/modules/notifications/notifications.module.ts Co-authored-by: Pierre Lehnen <55164754+pierre-lehnen-rc@users.noreply.github.com> --- .../server/modules/notifications/notifications.module.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apps/meteor/server/modules/notifications/notifications.module.ts b/apps/meteor/server/modules/notifications/notifications.module.ts index 94cd7d84a81c5..22b198bfa66f9 100644 --- a/apps/meteor/server/modules/notifications/notifications.module.ts +++ b/apps/meteor/server/modules/notifications/notifications.module.ts @@ -112,13 +112,11 @@ export class NotificationsModule { } if (room.t === 'c' && this.userId) { - if (await Authorization.hasPermission(this.userId, 'preview-c-room')) { + if (!(await Authorization.hasPermission(this.userId, 'preview-c-room'))) { const subscription = await Subscriptions.findOneByRoomIdAndUserId(room._id, this.userId); return !!subscription; } - - return false; } return true; From 597c22969a28fb1819a1ee615dbb7333dc83d2eb Mon Sep 17 00:00:00 2001 From: Tiago Evangelista Date: Thu, 13 Feb 2025 17:15:07 -0300 Subject: [PATCH 5/8] improve --- .../app/lib/server/methods/getChannelHistory.ts | 10 +++------- .../modules/notifications/notifications.module.ts | 10 +--------- .../server/services/authorization/canReadRoom.ts | 14 ++++++++++++++ .../server/services/authorization/service.ts | 5 +++++ packages/core-services/src/types/IAuthorization.ts | 1 + 5 files changed, 24 insertions(+), 16 deletions(-) create mode 100644 apps/meteor/server/services/authorization/canReadRoom.ts diff --git a/apps/meteor/app/lib/server/methods/getChannelHistory.ts b/apps/meteor/app/lib/server/methods/getChannelHistory.ts index 8fe5812dd62a1..97d1d801d5b12 100644 --- a/apps/meteor/app/lib/server/methods/getChannelHistory.ts +++ b/apps/meteor/app/lib/server/methods/getChannelHistory.ts @@ -1,11 +1,11 @@ +import { Authorization } from '@rocket.chat/core-services'; import type { IMessage, MessageTypesValues } from '@rocket.chat/core-typings'; import type { ServerMethods } from '@rocket.chat/ddp-client'; -import { Messages, Subscriptions, Rooms } from '@rocket.chat/models'; +import { Messages, Rooms } from '@rocket.chat/models'; import { check } from 'meteor/check'; import { Meteor } from 'meteor/meteor'; import { canAccessRoomAsync } from '../../../authorization/server'; -import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission'; import { settings } from '../../../settings/server/cached'; import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser'; import { getHiddenSystemMessages } from '../lib/getHiddenSystemMessages'; @@ -49,11 +49,7 @@ Meteor.methods({ } // Make sure they can access the room - if ( - room.t === 'c' && - !(await hasPermissionAsync(fromUserId, 'preview-c-room')) && - !(await Subscriptions.findOneByRoomIdAndUserId(rid, fromUserId, { projection: { _id: 1 } })) - ) { + if (!Authorization.canReadRoom(fromUserId, { roomType: room.t, rid: room._id })) { return false; } diff --git a/apps/meteor/server/modules/notifications/notifications.module.ts b/apps/meteor/server/modules/notifications/notifications.module.ts index 22b198bfa66f9..a70fc1cccf17c 100644 --- a/apps/meteor/server/modules/notifications/notifications.module.ts +++ b/apps/meteor/server/modules/notifications/notifications.module.ts @@ -111,15 +111,7 @@ export class NotificationsModule { return false; } - if (room.t === 'c' && this.userId) { - if (!(await Authorization.hasPermission(this.userId, 'preview-c-room'))) { - const subscription = await Subscriptions.findOneByRoomIdAndUserId(room._id, this.userId); - - return !!subscription; - } - } - - return true; + return Authorization.canReadRoom(this.userId || '', { roomType: room.t, rid: room._id }); }); this.streamRoomMessage.allowRead('__my_messages__', 'all'); diff --git a/apps/meteor/server/services/authorization/canReadRoom.ts b/apps/meteor/server/services/authorization/canReadRoom.ts new file mode 100644 index 0000000000000..29d583575a4e1 --- /dev/null +++ b/apps/meteor/server/services/authorization/canReadRoom.ts @@ -0,0 +1,14 @@ +import { Authorization } from '@rocket.chat/core-services'; +import { Subscriptions } from '@rocket.chat/models'; + +export const canReadRoom = async (uid: string, { roomType, rid }: { roomType: string; rid: string }) => { + if ( + roomType === 'c' && + !(await Authorization.hasPermission(uid, 'preview-c-room')) && + !(await Subscriptions.findOneByRoomIdAndUserId(rid, uid, { projection: { _id: 1 } })) + ) { + return false; + } + + return true; +}; diff --git a/apps/meteor/server/services/authorization/service.ts b/apps/meteor/server/services/authorization/service.ts index 5e67cf87b372c..ae38e5dad01d5 100644 --- a/apps/meteor/server/services/authorization/service.ts +++ b/apps/meteor/server/services/authorization/service.ts @@ -5,6 +5,7 @@ import { Subscriptions, Rooms, Users, Roles, Permissions } from '@rocket.chat/mo import mem from 'mem'; import { canAccessRoom } from './canAccessRoom'; +import { canReadRoom } from './canReadRoom'; import { AuthorizationUtils } from '../../../app/authorization/lib/AuthorizationUtils'; import './canAccessRoomLivechat'; @@ -80,6 +81,10 @@ export class Authorization extends ServiceClass implements IAuthorization { return canAccessRoom(...args); } + async canReadRoom(...args: Parameters): Promise { + return canReadRoom(...args); + } + async canAccessRoomId(rid: IRoom['_id'], uid: IUser['_id']): Promise { const room = await Rooms.findOneById>(rid, { projection: { diff --git a/packages/core-services/src/types/IAuthorization.ts b/packages/core-services/src/types/IAuthorization.ts index a6b744b17f2e2..58702835e5cfa 100644 --- a/packages/core-services/src/types/IAuthorization.ts +++ b/packages/core-services/src/types/IAuthorization.ts @@ -11,6 +11,7 @@ export interface IAuthorization { hasPermission(userId: string, permissionId: string, scope?: string): Promise; hasAtLeastOnePermission(userId: string, permissions: string[], scope?: string): Promise; canAccessRoom: RoomAccessValidator; + canReadRoom: (uid: IUser['_id'], room: { roomType: IRoom['t']; rid: IRoom['_id'] }) => Promise; canAccessRoomId(rid: IRoom['_id'], uid?: IUser['_id']): Promise; getUsersFromPublicRoles(): Promise<(IRocketChatRecord & Pick)[]>; } From c2b2173db7064ed5de21f75b7b4f3488aced5a23 Mon Sep 17 00:00:00 2001 From: Tiago Evangelista Date: Fri, 14 Feb 2025 15:28:38 -0300 Subject: [PATCH 6/8] improve again --- .../lib/server/methods/getChannelHistory.ts | 7 +------ .../notifications/notifications.module.ts | 6 +----- .../services/authorization/canReadRoom.ts | 18 ++++++++++++++---- .../server/services/authorization/service.ts | 4 ++-- packages/core-services/src/index.ts | 3 ++- .../core-services/src/types/IAuthorization.ts | 7 ++++++- 6 files changed, 26 insertions(+), 19 deletions(-) diff --git a/apps/meteor/app/lib/server/methods/getChannelHistory.ts b/apps/meteor/app/lib/server/methods/getChannelHistory.ts index 97d1d801d5b12..090ee73d05f23 100644 --- a/apps/meteor/app/lib/server/methods/getChannelHistory.ts +++ b/apps/meteor/app/lib/server/methods/getChannelHistory.ts @@ -5,7 +5,6 @@ import { Messages, Rooms } from '@rocket.chat/models'; import { check } from 'meteor/check'; import { Meteor } from 'meteor/meteor'; -import { canAccessRoomAsync } from '../../../authorization/server'; import { settings } from '../../../settings/server/cached'; import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser'; import { getHiddenSystemMessages } from '../lib/getHiddenSystemMessages'; @@ -44,12 +43,8 @@ Meteor.methods({ return false; } - if (!(await canAccessRoomAsync(room, { _id: fromUserId }))) { - return false; - } - // Make sure they can access the room - if (!Authorization.canReadRoom(fromUserId, { roomType: room.t, rid: room._id })) { + if (!(await Authorization.canReadRoom(room, { _id: fromUserId }))) { return false; } diff --git a/apps/meteor/server/modules/notifications/notifications.module.ts b/apps/meteor/server/modules/notifications/notifications.module.ts index a70fc1cccf17c..c11439d5861f4 100644 --- a/apps/meteor/server/modules/notifications/notifications.module.ts +++ b/apps/meteor/server/modules/notifications/notifications.module.ts @@ -104,14 +104,10 @@ export class NotificationsModule { const canAccess = await Authorization.canAccessRoom(room, { _id: this.userId || '' }, extraData); if (!canAccess) { - // verify if can preview messages from public channels - if (room.t === 'c' && this.userId) { - return Authorization.hasPermission(this.userId, 'preview-c-room'); - } return false; } - return Authorization.canReadRoom(this.userId || '', { roomType: room.t, rid: room._id }); + return Authorization.canReadRoom(room, { _id: this.userId || '' }); }); this.streamRoomMessage.allowRead('__my_messages__', 'all'); diff --git a/apps/meteor/server/services/authorization/canReadRoom.ts b/apps/meteor/server/services/authorization/canReadRoom.ts index 29d583575a4e1..165cc898a3dc1 100644 --- a/apps/meteor/server/services/authorization/canReadRoom.ts +++ b/apps/meteor/server/services/authorization/canReadRoom.ts @@ -1,11 +1,21 @@ +import type { RoomReadValidator } from '@rocket.chat/core-services'; import { Authorization } from '@rocket.chat/core-services'; import { Subscriptions } from '@rocket.chat/models'; -export const canReadRoom = async (uid: string, { roomType, rid }: { roomType: string; rid: string }) => { +import { canAccessRoom } from './canAccessRoom'; + +export const canReadRoom: RoomReadValidator = async (...args) => { + if (!(await canAccessRoom(...args))) { + return false; + } + + const [room, user] = args; + if ( - roomType === 'c' && - !(await Authorization.hasPermission(uid, 'preview-c-room')) && - !(await Subscriptions.findOneByRoomIdAndUserId(rid, uid, { projection: { _id: 1 } })) + user?._id && + room?.t === 'c' && + !(await Authorization.hasPermission(user._id, 'preview-c-room')) && + !(await Subscriptions.findOneByRoomIdAndUserId(room?._id, user._id, { projection: { _id: 1 } })) ) { return false; } diff --git a/apps/meteor/server/services/authorization/service.ts b/apps/meteor/server/services/authorization/service.ts index ae38e5dad01d5..71827281adbf0 100644 --- a/apps/meteor/server/services/authorization/service.ts +++ b/apps/meteor/server/services/authorization/service.ts @@ -1,4 +1,4 @@ -import type { IAuthorization, RoomAccessValidator } from '@rocket.chat/core-services'; +import type { IAuthorization, RoomAccessValidator, RoomReadValidator } from '@rocket.chat/core-services'; import { License, ServiceClass } from '@rocket.chat/core-services'; import type { IUser, IRole, IRoom, ISubscription, IRocketChatRecord } from '@rocket.chat/core-typings'; import { Subscriptions, Rooms, Users, Roles, Permissions } from '@rocket.chat/models'; @@ -81,7 +81,7 @@ export class Authorization extends ServiceClass implements IAuthorization { return canAccessRoom(...args); } - async canReadRoom(...args: Parameters): Promise { + async canReadRoom(...args: Parameters): Promise { return canReadRoom(...args); } diff --git a/packages/core-services/src/index.ts b/packages/core-services/src/index.ts index dedbe3571aaac..0c63532c4fe7d 100644 --- a/packages/core-services/src/index.ts +++ b/packages/core-services/src/index.ts @@ -3,7 +3,7 @@ import type { IAccount, ILoginResult } from './types/IAccount'; import type { IAnalyticsService } from './types/IAnalyticsService'; import { IApiService } from './types/IApiService'; import type { IAppsEngineService } from './types/IAppsEngineService'; -import type { IAuthorization, RoomAccessValidator } from './types/IAuthorization'; +import type { IAuthorization, RoomAccessValidator, RoomReadValidator } from './types/IAuthorization'; import type { IAuthorizationLivechat } from './types/IAuthorizationLivechat'; import type { IAuthorizationVoip } from './types/IAuthorizationVoip'; import type { IBannerService } from './types/IBannerService'; @@ -127,6 +127,7 @@ export { proxify, ResizeResult, RoomAccessValidator, + RoomReadValidator, TelemetryEvents, TelemetryMap, VideoConferenceJoinOptions, diff --git a/packages/core-services/src/types/IAuthorization.ts b/packages/core-services/src/types/IAuthorization.ts index 58702835e5cfa..a1e4e71736741 100644 --- a/packages/core-services/src/types/IAuthorization.ts +++ b/packages/core-services/src/types/IAuthorization.ts @@ -6,12 +6,17 @@ export type RoomAccessValidator = ( extraData?: Record, ) => Promise; +type Head = T extends [...infer U, any?] ? U : never; +type RemoveLastParameter any> = (...args: Head>) => ReturnType; + +export type RoomReadValidator = RemoveLastParameter; + export interface IAuthorization { hasAllPermission(userId: string, permissions: string[], scope?: string): Promise; hasPermission(userId: string, permissionId: string, scope?: string): Promise; hasAtLeastOnePermission(userId: string, permissions: string[], scope?: string): Promise; canAccessRoom: RoomAccessValidator; - canReadRoom: (uid: IUser['_id'], room: { roomType: IRoom['t']; rid: IRoom['_id'] }) => Promise; + canReadRoom: RoomReadValidator; canAccessRoomId(rid: IRoom['_id'], uid?: IUser['_id']): Promise; getUsersFromPublicRoles(): Promise<(IRocketChatRecord & Pick)[]>; } From 76c3c4f3660bf62e6403c4f8f435e3b40d37cb0e Mon Sep 17 00:00:00 2001 From: Tiago Evangelista Date: Tue, 18 Feb 2025 15:42:29 -0300 Subject: [PATCH 7/8] last review --- .../server/modules/notifications/notifications.module.ts | 8 +------- apps/meteor/server/services/authorization/canReadRoom.ts | 4 ++-- apps/meteor/server/services/authorization/service.ts | 4 ++-- packages/core-services/src/types/IAuthorization.ts | 7 +------ 4 files changed, 6 insertions(+), 17 deletions(-) diff --git a/apps/meteor/server/modules/notifications/notifications.module.ts b/apps/meteor/server/modules/notifications/notifications.module.ts index c11439d5861f4..ad6bacba354b2 100644 --- a/apps/meteor/server/modules/notifications/notifications.module.ts +++ b/apps/meteor/server/modules/notifications/notifications.module.ts @@ -101,13 +101,7 @@ export class NotificationsModule { return false; } - const canAccess = await Authorization.canAccessRoom(room, { _id: this.userId || '' }, extraData); - - if (!canAccess) { - return false; - } - - return Authorization.canReadRoom(room, { _id: this.userId || '' }); + return Authorization.canReadRoom(room, { _id: this.userId || '' }, extraData); }); this.streamRoomMessage.allowRead('__my_messages__', 'all'); diff --git a/apps/meteor/server/services/authorization/canReadRoom.ts b/apps/meteor/server/services/authorization/canReadRoom.ts index 165cc898a3dc1..73e4dd86e5bd7 100644 --- a/apps/meteor/server/services/authorization/canReadRoom.ts +++ b/apps/meteor/server/services/authorization/canReadRoom.ts @@ -1,10 +1,10 @@ -import type { RoomReadValidator } from '@rocket.chat/core-services'; +import type { RoomAccessValidator } from '@rocket.chat/core-services'; import { Authorization } from '@rocket.chat/core-services'; import { Subscriptions } from '@rocket.chat/models'; import { canAccessRoom } from './canAccessRoom'; -export const canReadRoom: RoomReadValidator = async (...args) => { +export const canReadRoom: RoomAccessValidator = async (...args) => { if (!(await canAccessRoom(...args))) { return false; } diff --git a/apps/meteor/server/services/authorization/service.ts b/apps/meteor/server/services/authorization/service.ts index 71827281adbf0..d970068e23ba4 100644 --- a/apps/meteor/server/services/authorization/service.ts +++ b/apps/meteor/server/services/authorization/service.ts @@ -1,4 +1,4 @@ -import type { IAuthorization, RoomAccessValidator, RoomReadValidator } from '@rocket.chat/core-services'; +import type { IAuthorization, RoomAccessValidator } from '@rocket.chat/core-services'; import { License, ServiceClass } from '@rocket.chat/core-services'; import type { IUser, IRole, IRoom, ISubscription, IRocketChatRecord } from '@rocket.chat/core-typings'; import { Subscriptions, Rooms, Users, Roles, Permissions } from '@rocket.chat/models'; @@ -81,7 +81,7 @@ export class Authorization extends ServiceClass implements IAuthorization { return canAccessRoom(...args); } - async canReadRoom(...args: Parameters): Promise { + async canReadRoom(...args: Parameters): Promise { return canReadRoom(...args); } diff --git a/packages/core-services/src/types/IAuthorization.ts b/packages/core-services/src/types/IAuthorization.ts index a1e4e71736741..eb58a01dc8d7e 100644 --- a/packages/core-services/src/types/IAuthorization.ts +++ b/packages/core-services/src/types/IAuthorization.ts @@ -6,17 +6,12 @@ export type RoomAccessValidator = ( extraData?: Record, ) => Promise; -type Head = T extends [...infer U, any?] ? U : never; -type RemoveLastParameter any> = (...args: Head>) => ReturnType; - -export type RoomReadValidator = RemoveLastParameter; - export interface IAuthorization { hasAllPermission(userId: string, permissions: string[], scope?: string): Promise; hasPermission(userId: string, permissionId: string, scope?: string): Promise; hasAtLeastOnePermission(userId: string, permissions: string[], scope?: string): Promise; canAccessRoom: RoomAccessValidator; - canReadRoom: RoomReadValidator; + canReadRoom: RoomAccessValidator; canAccessRoomId(rid: IRoom['_id'], uid?: IUser['_id']): Promise; getUsersFromPublicRoles(): Promise<(IRocketChatRecord & Pick)[]>; } From accad371c061db465109b2379a6839a0c44bd55f Mon Sep 17 00:00:00 2001 From: Tiago Evangelista Date: Thu, 20 Feb 2025 07:57:07 -0300 Subject: [PATCH 8/8] no exported member --- packages/core-services/src/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core-services/src/index.ts b/packages/core-services/src/index.ts index 0c63532c4fe7d..dedbe3571aaac 100644 --- a/packages/core-services/src/index.ts +++ b/packages/core-services/src/index.ts @@ -3,7 +3,7 @@ import type { IAccount, ILoginResult } from './types/IAccount'; import type { IAnalyticsService } from './types/IAnalyticsService'; import { IApiService } from './types/IApiService'; import type { IAppsEngineService } from './types/IAppsEngineService'; -import type { IAuthorization, RoomAccessValidator, RoomReadValidator } from './types/IAuthorization'; +import type { IAuthorization, RoomAccessValidator } from './types/IAuthorization'; import type { IAuthorizationLivechat } from './types/IAuthorizationLivechat'; import type { IAuthorizationVoip } from './types/IAuthorizationVoip'; import type { IBannerService } from './types/IBannerService'; @@ -127,7 +127,6 @@ export { proxify, ResizeResult, RoomAccessValidator, - RoomReadValidator, TelemetryEvents, TelemetryMap, VideoConferenceJoinOptions,