From 8984df841e1ecc46198e073982adabc1f069c352 Mon Sep 17 00:00:00 2001 From: Nazareno Bucciarelli <84046180+nazabucciarelli@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:10:03 +0000 Subject: [PATCH 1/4] fix: hidden system messages being counted on discussions (#41673) --- .changeset/sour-pugs-bow.md | 8 +++ .../Info/EditRoomInfo/EditRoomInfo.tsx | 12 +++- .../messages/propagateDiscussionMetadata.ts | 2 + ...ateAndNotifyParentRoomWithParentMessage.ts | 44 +++++++++++- .../meteor-methods/rooms/saveRoomSettings.ts | 9 +++ .../server/services/messages/service.ts | 2 +- apps/meteor/tests/end-to-end/api/rooms.ts | 70 ++++++++++++++----- packages/i18n/src/locales/en.i18n.json | 1 + .../src/models/IMessagesModel.ts | 1 + packages/models/src/models/Messages.ts | 23 ++++-- 10 files changed, 145 insertions(+), 27 deletions(-) create mode 100644 .changeset/sour-pugs-bow.md diff --git a/.changeset/sour-pugs-bow.md b/.changeset/sour-pugs-bow.md new file mode 100644 index 0000000000000..b57ab03aa431a --- /dev/null +++ b/.changeset/sour-pugs-bow.md @@ -0,0 +1,8 @@ +--- +'@rocket.chat/model-typings': patch +'@rocket.chat/models': patch +'@rocket.chat/meteor': patch +'@rocket.chat/i18n': patch +--- + +Fixes the messages count displayed on a discussion taking into account system messages which are hidden inside of it, making the count higher than the number of messages actually visible after opening the discussion. The count now excludes every system message type hidden either globally or on the discussion itself. A hint was also added to the `Hide system messages` option of the room edit panel clarifying that the hidden messages are not included in the count. diff --git a/apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx b/apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx index c89b9e298c65c..620456dff688b 100644 --- a/apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx +++ b/apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx @@ -475,7 +475,13 @@ const EditRoomInfo = ({ room, onClickClose, onClickBack }: EditRoomInfoProps) => control={control} name='hideSysMes' render={({ field: { value, ...field } }) => ( - + )} /> @@ -490,10 +496,14 @@ const EditRoomInfo = ({ room, onClickClose, onClickBack }: EditRoomInfoProps) => disabled={!hideSysMes || isFederated} placeholder={t('Select_messages_to_hide')} aria-label={t('Select_messages_to_hide')} + aria-describedby={`${hideSysMesField}-hint`} /> )} /> + + {t('Hide_System_Messages_Hint')} + )} diff --git a/apps/meteor/server/hooks/messages/propagateDiscussionMetadata.ts b/apps/meteor/server/hooks/messages/propagateDiscussionMetadata.ts index 370a4075b75fd..389d034b5f600 100644 --- a/apps/meteor/server/hooks/messages/propagateDiscussionMetadata.ts +++ b/apps/meteor/server/hooks/messages/propagateDiscussionMetadata.ts @@ -39,6 +39,7 @@ callbacks.add( projection: { msgs: 1, lm: 1, + sysMes: 1, }, }); @@ -62,6 +63,7 @@ callbacks.add( projection: { msgs: 1, lm: 1, + sysMes: 1, }, }); diff --git a/apps/meteor/server/lib/messaging/discussions/updateAndNotifyParentRoomWithParentMessage.ts b/apps/meteor/server/lib/messaging/discussions/updateAndNotifyParentRoomWithParentMessage.ts index a231b602f15e7..02bb51f9a8cee 100644 --- a/apps/meteor/server/lib/messaging/discussions/updateAndNotifyParentRoomWithParentMessage.ts +++ b/apps/meteor/server/lib/messaging/discussions/updateAndNotifyParentRoomWithParentMessage.ts @@ -1,13 +1,53 @@ -import type { IRoom } from '@rocket.chat/core-typings'; +import type { IRoom, MessageTypesValues } from '@rocket.chat/core-typings'; import { Messages } from '@rocket.chat/models'; +import { settings } from '../../../settings/cached'; import { notifyOnMessageChange } from '../../notifyListener'; -export const updateAndNotifyParentRoomWithParentMessage = async (room: IRoom): Promise => { +type DiscussionRoom = Pick; + +/** + * Both the system messages hidden globally and the ones hidden on the room itself are + * filtered out from the room history, so the count has to consider both of them. + * Type `rm` is filtered out because it's already discounted when the message is deleted. + */ +const getHiddenTypesToDiscount = (room: DiscussionRoom): MessageTypesValues[] => { + const globalHiddenTypes = settings.get('Hide_System_Messages'); + const globallyHiddenTypes = Array.isArray(globalHiddenTypes) ? globalHiddenTypes : []; + const roomHiddenTypes = Array.isArray(room.sysMes) ? room.sysMes : []; + + return [...new Set([...globallyHiddenTypes, ...roomHiddenTypes])] + .flatMap((type) => + // `mute_unmute` is a single option covering two different message types + type === 'mute_unmute' ? ['user-muted', 'user-unmuted'] : [type], + ) + .filter((type) => type !== 'rm'); +}; + +const getDiscussionMessagesCount = async (room: DiscussionRoom): Promise => { + const hiddenMessageTypes = getHiddenTypesToDiscount(room); + + if (!hiddenMessageTypes.length) { + return room.msgs; + } + + const hiddenMessagesCount = await Messages.countVisibleByRoomIdContainingTypes(room._id, hiddenMessageTypes); + + return Math.max(room.msgs - hiddenMessagesCount, 0); +}; + +/** + * Copies the current metadata of a discussion (messages count and last message timestamp) to the + * message which links to it on the parent room, and notifies the change to the clients. + */ +export const updateAndNotifyParentRoomWithParentMessage = async (room: DiscussionRoom): Promise => { + room.msgs = await getDiscussionMessagesCount(room); + const parentMessage = await Messages.refreshDiscussionMetadata(room); if (!parentMessage) { return; } + void notifyOnMessageChange({ id: parentMessage._id, data: parentMessage, diff --git a/apps/meteor/server/meteor-methods/rooms/saveRoomSettings.ts b/apps/meteor/server/meteor-methods/rooms/saveRoomSettings.ts index be5af6280c90a..59a6e845fec87 100644 --- a/apps/meteor/server/meteor-methods/rooms/saveRoomSettings.ts +++ b/apps/meteor/server/meteor-methods/rooms/saveRoomSettings.ts @@ -9,6 +9,8 @@ import { Meteor } from 'meteor/meteor'; import { RoomSettingsEnum } from '../../../definition/IRoomTypeConfig'; import { hasPermissionAsync } from '../../lib/authorization/hasPermission'; import { isABACManagedRoom } from '../../lib/authorization/isABACManagedRoom'; +import { SystemLogger } from '../../lib/logger/system'; +import { updateAndNotifyParentRoomWithParentMessage } from '../../lib/messaging/discussions/updateAndNotifyParentRoomWithParentMessage'; import { notifyOnRoomChangedById } from '../../lib/notifyListener'; import { roomCoordinator } from '../../lib/rooms/roomCoordinator'; import { setRoomAvatar } from '../../lib/rooms/setRoomAvatar'; @@ -334,6 +336,13 @@ const settingSavers: RoomSettingsSavers = { async systemMessages({ value, room, rid }) { if (JSON.stringify(value) !== JSON.stringify(room.sysMes)) { await saveRoomSystemMessages(rid, value); + if (room?.prid) { + try { + await updateAndNotifyParentRoomWithParentMessage({ ...room, sysMes: value }); + } catch (err) { + SystemLogger.error({ msg: 'Failed to propagate discussion metadata', err, rid }); + } + } } }, async joinCode({ value, rid }) { diff --git a/apps/meteor/server/services/messages/service.ts b/apps/meteor/server/services/messages/service.ts index 99e65e59f995f..c12d8330119c2 100644 --- a/apps/meteor/server/services/messages/service.ts +++ b/apps/meteor/server/services/messages/service.ts @@ -196,7 +196,7 @@ export class MessageService extends ServiceClassInternal implements IMessageServ settings.get('Message_Read_Receipt_Enabled'), extraData, ), - Rooms.findOneAndIncMsgCountById(rid, 1, { projection: { prid: 1, msgs: 1, lm: 1 } }), + Rooms.findOneAndIncMsgCountById(rid, 1, { projection: { prid: 1, msgs: 1, lm: 1, sysMes: 1 } }), ]); if (!insertedId) { diff --git a/apps/meteor/tests/end-to-end/api/rooms.ts b/apps/meteor/tests/end-to-end/api/rooms.ts index 022b6434b1e70..1612e8e3d0b05 100644 --- a/apps/meteor/tests/end-to-end/api/rooms.ts +++ b/apps/meteor/tests/end-to-end/api/rooms.ts @@ -2070,6 +2070,14 @@ describe('[Rooms]', () => { return body.messages.find((message: IMessage & { drid: IRoom['_id'] }) => message.drid === discussion._id); }; + const saveDiscussionSettings = (settings: Record) => + request + .post(api('rooms.saveRoomSettings')) + .set(credentials) + .send({ rid: discussion._id, ...settings }) + .expect('Content-Type', 'application/json') + .expect(200); + beforeEach(async () => { testChannel = (await createRoom({ type: 'c', name: `channel.test.${Date.now()}-${Math.random()}` })).body.channel; @@ -2085,27 +2093,55 @@ describe('[Rooms]', () => { // deleting the parent channel also deletes its discussions afterEach(() => deleteRoom({ type: 'c', roomId: testChannel._id })); - it('should count the message just sent on the discussion', async () => { - const sentMessage = await sendSimpleMessage({ roomId: discussion._id }); - const discussionMessage = await getDiscussionMessage(); + describe('with no system message hidden', () => { + it('should count the message just sent on the discussion', async () => { + const sentMessage = await sendSimpleMessage({ roomId: discussion._id }); + const discussionMessage = await getDiscussionMessage(); + + expect(discussionMessage).to.have.property('dcount', 1); + expect(discussionMessage).to.have.property('dlm', sentMessage.body.message.ts); + }); - expect(discussionMessage).to.have.property('dcount', 1); - expect(discussionMessage).to.have.property('dlm', sentMessage.body.message.ts); + it('should count the system messages of the discussion', async () => { + await saveDiscussionSettings({ roomName: `edited-discussion-name-${Date.now()}` }); + expect(await getDiscussionMessage()).to.have.property('dcount', 1); + }); }); - it('should count the system message just sent on the discussion', async () => { - await request - .post(api('rooms.saveRoomSettings')) - .set(credentials) - .send({ - rid: discussion._id, - roomName: 'edited-discussion-name', - }) - .expect('Content-Type', 'application/json') - .expect(200); - const discussionMessage = await getDiscussionMessage(); + describe('with system messages hidden on the discussion', () => { + beforeEach(() => saveDiscussionSettings({ systemMessages: ['r'] })); + + it('should not count the hidden system messages', async () => { + await saveDiscussionSettings({ roomName: `edited-discussion-name-${Date.now()}` }); + await sendSimpleMessage({ roomId: discussion._id }); + + expect(await getDiscussionMessage()).to.have.property('dcount', 1); + }); + + it('should count them again once they are not hidden anymore', async () => { + await saveDiscussionSettings({ roomName: `edited-discussion-name-${Date.now()}` }); + expect(await getDiscussionMessage()).to.have.property('dcount', 0); + + await saveDiscussionSettings({ systemMessages: [] }); + + expect(await getDiscussionMessage()).to.have.property('dcount', 1); + }); + }); + + describe('with system messages hidden by the global setting', () => { + before(() => updateSetting('Hide_System_Messages', ['r'])); + + // the setting applies to the whole workspace, so it has to be restored + after(() => updateSetting('Hide_System_Messages', [])); - expect(discussionMessage).to.have.property('dcount', 1); + it('should not count the hidden system messages', async () => { + await saveDiscussionSettings({ roomName: `edited-discussion-name-${Date.now()}` }); + expect(await getDiscussionMessage()).to.have.property('dcount', 0); + + await sendSimpleMessage({ roomId: discussion._id }); + + expect(await getDiscussionMessage()).to.have.property('dcount', 1); + }); }); }); diff --git a/packages/i18n/src/locales/en.i18n.json b/packages/i18n/src/locales/en.i18n.json index 27cfc5173c289..5117925e00379 100644 --- a/packages/i18n/src/locales/en.i18n.json +++ b/packages/i18n/src/locales/en.i18n.json @@ -2649,6 +2649,7 @@ "Hide_Private_Warning": "Are you sure you want to hide the discussion with \"{{roomName}}\"?", "Hide_Room_Warning": "Are you sure you want to hide the channel \"{{roomName}}\"?", "Hide_System_Messages": "Hide system messages", + "Hide_System_Messages_Hint": "Hidden system messages are excluded from a room's message count.", "Hide_Unread_Room_Status": "Hide Unread Room Status", "Hide_additional_fields": "Hide additional fields", "Hide_counter": "Hide counter", diff --git a/packages/model-typings/src/models/IMessagesModel.ts b/packages/model-typings/src/models/IMessagesModel.ts index dc1f0147bab17..184d016e2655a 100644 --- a/packages/model-typings/src/models/IMessagesModel.ts +++ b/packages/model-typings/src/models/IMessagesModel.ts @@ -140,6 +140,7 @@ export interface IMessagesModel extends IBaseModel { options?: FindOptions, showThreadMessages?: boolean, ): FindCursor; + countVisibleByRoomIdContainingTypes(roomId: string, types: MessageTypesValues[]): Promise; findFilesByRoomIdPinnedTimestampAndUsers( rid: string, excludePinned: boolean, diff --git a/packages/models/src/models/Messages.ts b/packages/models/src/models/Messages.ts index 650bb94e3dbf1..4a288560e912e 100644 --- a/packages/models/src/models/Messages.ts +++ b/packages/models/src/models/Messages.ts @@ -762,6 +762,18 @@ export class MessagesRaw extends BaseRaw implements IMessagesModel { return this.find(query, options); } + countVisibleByRoomIdContainingTypes(roomId: string, types: MessageTypesValues[]): Promise { + const query: Filter = { + _hidden: { + $ne: true, + }, + rid: roomId, + t: { $in: types }, + }; + + return this.countDocuments(query); + } + findVisibleByRoomIdAfterTimestamp( roomId: string, timestamp: Date, @@ -1564,17 +1576,16 @@ export class MessagesRaw extends BaseRaw implements IMessagesModel { } /** - * Copy metadata from the discussion to the system message in the parent channel - * which links to the discussion. - * Since we don't pass this metadata into the model's function, it is not a subject - * to race conditions: If multiple updates occur, the current state will be updated - * only if the new state of the discussion room is really newer. + * Copy metadata from the discussion to the message in the parent channel which links to it. + * The metadata is received from the caller, so the update is guarded to not overwrite the + * parent message with an older state of the discussion room. */ async refreshDiscussionMetadata(room: Pick): Promise> { const { _id: drid, msgs: dcount, lm: dlm } = room; const query = { drid, + $or: [{ dlm: { $exists: false } }, ...(dlm ? [{ dlm: { $lte: dlm } }] : [])], }; return this.findOneAndUpdate( @@ -1582,7 +1593,7 @@ export class MessagesRaw extends BaseRaw implements IMessagesModel { { $set: { dcount, - dlm, + ...(dlm && { dlm }), }, }, { returnDocument: 'after' }, From 6d52e8e757b1d6cbd91f9f47339200f9f8dc8e73 Mon Sep 17 00:00:00 2001 From: Yash Rajpal <58601732+yash-rajpal@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:15:11 +0000 Subject: [PATCH 2/4] feat: SAML deeplink auth (#41788) --- .changeset/cute-pumas-trade.md | 5 ++ .../meteor/client/lib/buildAuthDeeplinkURL.ts | 7 ++ apps/meteor/client/meteor/login/saml.ts | 10 ++- .../client/views/root/SAMLLoginRoute.spec.tsx | 65 ++++++++++++++++++- .../client/views/root/SAMLLoginRoute.tsx | 16 ++++- apps/meteor/server/api/v1/settings.ts | 7 +- apps/meteor/server/lib/saml/lib/SAML.ts | 20 ++++-- .../server/lib/saml/lib/ServiceProvider.ts | 8 +-- apps/meteor/server/lib/saml/lib/Utils.ts | 31 +++++++++ apps/meteor/tests/e2e/saml.spec.ts | 61 +++++++++++++++++ .../unit/server/lib/saml/server.tests.ts | 31 +++++++++ .../web-ui-registration/src/LoginServices.tsx | 2 +- 12 files changed, 247 insertions(+), 16 deletions(-) create mode 100644 .changeset/cute-pumas-trade.md diff --git a/.changeset/cute-pumas-trade.md b/.changeset/cute-pumas-trade.md new file mode 100644 index 0000000000000..7bb0069a6ef75 --- /dev/null +++ b/.changeset/cute-pumas-trade.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': minor +--- + +Adds support for SAML authentication in the mobile and desktop apps via the system browser. diff --git a/apps/meteor/client/lib/buildAuthDeeplinkURL.ts b/apps/meteor/client/lib/buildAuthDeeplinkURL.ts index e4481fed271d3..2598a073b7eb1 100644 --- a/apps/meteor/client/lib/buildAuthDeeplinkURL.ts +++ b/apps/meteor/client/lib/buildAuthDeeplinkURL.ts @@ -3,3 +3,10 @@ export const buildDeepLinkURL = (resumeToken: string, userId: string) => { const { origin } = url; return `rocketchat://auth?host=${origin}&token=${resumeToken}&userId=${userId}`; }; + +export const buildSamlDeepLinkURL = (credentialToken: string) => { + const { origin } = new URL(window.location.href); + const params = new URLSearchParams({ type: 'saml', host: origin, credentialToken }); + + return `rocketchat://auth?${params.toString()}`; +}; diff --git a/apps/meteor/client/meteor/login/saml.ts b/apps/meteor/client/meteor/login/saml.ts index 1dcb48d91f067..b530afdee1d36 100644 --- a/apps/meteor/client/meteor/login/saml.ts +++ b/apps/meteor/client/meteor/login/saml.ts @@ -91,7 +91,15 @@ Meteor.loginWithSaml = (options) => { const credentialToken = `id-${Random.id()}`; options.credentialToken = credentialToken; - window.location.href = `_saml/authorize/${options.provider}/${options.credentialToken}`; + let url = `_saml/authorize/${options.provider}/${options.credentialToken}`; + + // Forward the loginClient so the session can be deep-linked back to the native client. + const loginClient = new URLSearchParams(window.location.search).get('loginClient'); + if (settings.peek('Accounts_OAuth_Use_Modern_Flow') && (loginClient === 'desktop' || loginClient === 'mobile')) { + url += `?loginClient=${loginClient}`; + } + + window.location.href = url; }; const loginWithSamlToken = (credentialToken: string) => diff --git a/apps/meteor/client/views/root/SAMLLoginRoute.spec.tsx b/apps/meteor/client/views/root/SAMLLoginRoute.spec.tsx index 6a2c5a4c0b221..e1c4abd715ba2 100644 --- a/apps/meteor/client/views/root/SAMLLoginRoute.spec.tsx +++ b/apps/meteor/client/views/root/SAMLLoginRoute.spec.tsx @@ -1,4 +1,4 @@ -import { MockedServerContext, MockedUserContext } from '@rocket.chat/mock-providers'; +import { MockedServerContext, MockedSettingsContext, MockedUserContext } from '@rocket.chat/mock-providers'; import { render } from '@testing-library/react'; import { Meteor } from 'meteor/meteor'; @@ -89,3 +89,66 @@ it('should call loginWithSamlToken with the token when it is present', async () expect(Meteor.loginWithSamlToken).toHaveBeenCalledTimes(1); expect(Meteor.loginWithSamlToken).toHaveBeenLastCalledWith('testToken', expect.any(Function)); }); + +jest.mock('../../lib/buildAuthDeeplinkURL', () => ({ buildSamlDeepLinkURL: jest.fn() })); + +describe('native client handoff', () => { + const { buildSamlDeepLinkURL } = jest.requireMock('../../lib/buildAuthDeeplinkURL') as { buildSamlDeepLinkURL: jest.Mock }; + + const navigableDeepLink = `${window.location.origin}/#deep-link`; + + beforeEach(() => { + mockUseSamlInviteToken.mockReturnValue([null, () => ({})]); + buildSamlDeepLinkURL.mockReturnValue(navigableDeepLink); + }); + + afterEach(() => { + window.location.hash = ''; + }); + + it.each(['mobile', 'desktop'])('should hand the credential token to the %s client without logging in', async (loginClient) => { + render( + + + + + , + ); + + // The browser must never authenticate: the app redeems the credential on its own connection. + expect(Meteor.loginWithSamlToken).not.toHaveBeenCalled(); + expect(navigateStub).not.toHaveBeenCalled(); + + expect(buildSamlDeepLinkURL).toHaveBeenCalledTimes(1); + expect(buildSamlDeepLinkURL).toHaveBeenLastCalledWith('testToken'); + expect(window.location.href).toBe(navigableDeepLink); + }); + + it('should log in normally for an unrecognized loginClient', async () => { + render( + + + + + , + ); + + expect(buildSamlDeepLinkURL).not.toHaveBeenCalled(); + expect(Meteor.loginWithSamlToken).toHaveBeenCalledTimes(1); + }); + + it('should ignore loginClient and log in normally when the modern flow is disabled', async () => { + render( + + + + + + + , + ); + + expect(buildSamlDeepLinkURL).not.toHaveBeenCalled(); + expect(Meteor.loginWithSamlToken).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/meteor/client/views/root/SAMLLoginRoute.tsx b/apps/meteor/client/views/root/SAMLLoginRoute.tsx index 35e3c26db504b..5ed8347725ab8 100644 --- a/apps/meteor/client/views/root/SAMLLoginRoute.tsx +++ b/apps/meteor/client/views/root/SAMLLoginRoute.tsx @@ -1,17 +1,29 @@ -import { useRouter, useToastMessageDispatch } from '@rocket.chat/ui-contexts'; +import { useRouter, useToastMessageDispatch, useSearchParameter, useSetting } from '@rocket.chat/ui-contexts'; import { Meteor } from 'meteor/meteor'; import { useEffect } from 'react'; +import { buildSamlDeepLinkURL } from '../../lib/buildAuthDeeplinkURL'; import { useSamlInviteToken } from '../invite/hooks/useSamlInviteToken'; const SAMLLoginRoute = () => { const router = useRouter(); const dispatchToastMessage = useToastMessageDispatch(); const [inviteToken] = useSamlInviteToken(); + const loginClient = useSearchParameter('loginClient'); + const enableModernOAuthFlow = useSetting('Accounts_OAuth_Use_Modern_Flow', true); useEffect(() => { const { token } = router.getRouteParameters(); + //SAML token handoff to the native client (mobile/desktop) + if (enableModernOAuthFlow && (loginClient === 'desktop' || loginClient === 'mobile')) { + window.location.href = buildSamlDeepLinkURL(token); + const timeout = setTimeout(() => { + router.navigate('/home', { replace: true }); + }, 0); + return () => clearTimeout(timeout); + } + Meteor.loginWithSamlToken(token, (error?: unknown) => { if (error) { dispatchToastMessage({ type: 'error', message: error }); @@ -33,7 +45,7 @@ const SAMLLoginRoute = () => { ); } }); - }, [dispatchToastMessage, inviteToken, router]); + }, [dispatchToastMessage, enableModernOAuthFlow, inviteToken, loginClient, router]); return null; }; diff --git a/apps/meteor/server/api/v1/settings.ts b/apps/meteor/server/api/v1/settings.ts index c0f01dbe4e59b..7d7076e0c2038 100644 --- a/apps/meteor/server/api/v1/settings.ts +++ b/apps/meteor/server/api/v1/settings.ts @@ -197,11 +197,14 @@ API.v1.get( return { ...service, hideButtonOnMobile: false }; } - if (service.service && ['saml', 'cas', 'ldap'].includes(service.service)) { + if (service.service && ['cas', 'ldap'].includes(service.service)) { return { ...service, hideButtonOnMobile: false }; } - if ((service as OAuthConfiguration).custom || (service.service && service.service === 'wordpress')) { + if ( + (service as OAuthConfiguration).custom || + (service.service && (service.service === 'wordpress' || service.service === 'saml')) + ) { return { ...service, hideButtonOnMobile: isPassportFlowEnabled }; } diff --git a/apps/meteor/server/lib/saml/lib/SAML.ts b/apps/meteor/server/lib/saml/lib/SAML.ts index ab84649c1734d..406e0a4e820b0 100644 --- a/apps/meteor/server/lib/saml/lib/SAML.ts +++ b/apps/meteor/server/lib/saml/lib/SAML.ts @@ -77,7 +77,7 @@ export class SAML { case 'sloRedirect': return this.processSLORedirectAction(req, res, service); case 'authorize': - return this.processAuthorizeAction(res, service, samlObject); + return this.processAuthorizeAction(req, res, service, samlObject); case 'validate': return this.processValidateAction(req, res, service, samlObject); default: @@ -247,7 +247,6 @@ export class SAML { if ((username && username !== user.username) || (nameOverwrite && fullName && fullName !== user.name)) { await saveUserIdentity({ _id: user._id, name: nameOverwrite ? fullName || undefined : user.name, username }); } - // sending token along with the userId return { userId: user._id, @@ -457,15 +456,18 @@ export class SAML { } private static async processAuthorizeAction( + req: IIncomingMessage, res: ServerResponse, service: IServiceProviderOptions, samlObject: ISAMLAction, ): Promise { const serviceProvider = new SAMLServiceProvider(service); let url: string | undefined; + const requestedLoginClient = settings.get('Accounts_OAuth_Use_Modern_Flow') ? req.query.loginClient : undefined; + const loginClient = SAMLUtils.isSupportedLoginClient(requestedLoginClient) ? requestedLoginClient : undefined; try { - url = await serviceProvider.getAuthorizeUrl(samlObject.credentialToken); + url = await serviceProvider.getAuthorizeUrl(samlObject.credentialToken, loginClient); } catch (err: any) { SAMLUtils.error({ err, msg: 'Unable to generate authorize url' }); url = Meteor.absoluteUrl(); @@ -498,7 +500,9 @@ export class SAML { } const serviceProvider = new SAMLServiceProvider(service); - SAMLUtils.relayState = envelope.relayState ?? null; + const { provider, loginClient } = SAMLUtils.decodeAuthorizeRelayState(envelope.relayState); + // Keep exposing the provider as the relay state for downstream profile mapping. + SAMLUtils.relayState = provider ?? null; serviceProvider.validateResponse(envelope, async (err, profile /* , loggedOut*/) => { try { if (err) { @@ -534,7 +538,13 @@ export class SAML { }; await this.storeCredential(credentialToken, loginResult); - const url = Meteor.absoluteUrl(SAMLUtils.getValidationActionRedirectPath(credentialToken)); + + let redirectPath = SAMLUtils.getValidationActionRedirectPath(credentialToken); + if (loginClient) { + redirectPath += `&loginClient=${loginClient}`; + } + + const url = Meteor.absoluteUrl(redirectPath); redirect(url); } catch (err) { SAMLUtils.error({ err }); diff --git a/apps/meteor/server/lib/saml/lib/ServiceProvider.ts b/apps/meteor/server/lib/saml/lib/ServiceProvider.ts index 3157a11366a7c..346a36b674250 100644 --- a/apps/meteor/server/lib/saml/lib/ServiceProvider.ts +++ b/apps/meteor/server/lib/saml/lib/ServiceProvider.ts @@ -119,7 +119,7 @@ export class SAMLServiceProvider { /* This method will generate the request URL with all the query string params and pass it to the callback */ - public async requestToUrl(request: string, operation: string): Promise { + public async requestToUrl(request: string, operation: string, loginClient?: string): Promise { const buffer = await util.promisify(zlib.deflateRaw)(request); try { const base64 = buffer.toString('base64'); @@ -143,7 +143,7 @@ export class SAMLServiceProvider { // in case of logout we want to be redirected back to the Meteor app. relayState = Meteor.absoluteUrl(); } else { - relayState = this.serviceProviderOptions.provider; + relayState = SAMLUtils.encodeAuthorizeRelayState(this.serviceProviderOptions.provider, loginClient); } const samlRequest = this.maybeSignRequest({ @@ -165,11 +165,11 @@ export class SAMLServiceProvider { } } - public async getAuthorizeUrl(credentialToken: string): Promise { + public async getAuthorizeUrl(credentialToken: string, loginClient?: string): Promise { const request = this.generateAuthorizeRequest(credentialToken); SAMLUtils.log({ request, msg: 'getAuthorizeUrl' }); - return this.requestToUrl(request, 'authorize'); + return this.requestToUrl(request, 'authorize', loginClient); } public async validateLogoutRequest( diff --git a/apps/meteor/server/lib/saml/lib/Utils.ts b/apps/meteor/server/lib/saml/lib/Utils.ts index 6a540e56e266d..5e5ded5c28443 100644 --- a/apps/meteor/server/lib/saml/lib/Utils.ts +++ b/apps/meteor/server/lib/saml/lib/Utils.ts @@ -137,6 +137,37 @@ export class SAMLUtils { return `saml/${credentialToken}?saml_idp_credentialToken=${credentialToken}`; } + public static isSupportedLoginClient(value: unknown): value is 'desktop' | 'mobile' { + return value === 'desktop' || value === 'mobile'; + } + + public static encodeAuthorizeRelayState(provider: string, loginClient?: string): string { + if (!this.isSupportedLoginClient(loginClient)) { + return provider; + } + + return new URLSearchParams({ provider, loginClient }).toString(); + } + + public static decodeAuthorizeRelayState(relayState?: string | null): { provider?: string; loginClient?: 'desktop' | 'mobile' } { + if (!relayState) { + return {}; + } + + if (relayState.startsWith('provider=') && relayState.includes('&loginClient=')) { + const params = new URLSearchParams(relayState); + const provider = params.get('provider') ?? undefined; + const loginClient = params.get('loginClient'); + + return { + provider, + loginClient: this.isSupportedLoginClient(loginClient) ? loginClient : undefined, + }; + } + + return { provider: relayState }; + } + public static log(obj: object | string): void { if (debug && logger) { logger.debug(obj); diff --git a/apps/meteor/tests/e2e/saml.spec.ts b/apps/meteor/tests/e2e/saml.spec.ts index da4ec228fba0b..07bff7fd8fac8 100644 --- a/apps/meteor/tests/e2e/saml.spec.ts +++ b/apps/meteor/tests/e2e/saml.spec.ts @@ -808,6 +808,67 @@ test.describe('SAML', () => { }); }); + test.describe('SAML Login handoff', () => { + // When a native client starts the login it passes `loginClient`, and the web client must hand the SAML + // credential token over to the app instead of logging itself in, so that only one session is ever created. + const findCredentialToken = async (credentialToken: string) => { + const connection = await MongoClient.connect(constants.URL_MONGODB); + try { + return await connection + .db() + .collection<{ _id: string; userInfo?: { profile?: Record } }>('rocketchat_credential_tokens') + .findOne({ _id: credentialToken }); + } finally { + await connection.close(); + } + }; + + test.beforeAll(async ({ api }) => { + await api.post('/settings/Accounts_OAuth_Use_Modern_Flow', { value: true }); + }); + + test.afterAll(async ({ api }) => { + await api.post('/settings/Accounts_OAuth_Use_Modern_Flow', { value: false }); + }); + + test('Hand the credential token to the desktop client without logging in the browser', async ({ page }) => { + await page.goto('/home?loginClient=desktop'); + + await expect(page).toHaveURL(/loginClient=desktop/); + + // Passing null skips the logged-in assertions, since the browser must not get a session here. + await doLoginStep(page, 'samluser1', null); + + let credentialToken: string | null = null; + + await test.step('expect to land on the SAML handoff route carrying the credential token', async () => { + // SAMLLoginRoute redirects to rocketchat://auth from here. The browser has no handler for that + // scheme, so the page stays put and we can inspect exactly what would have been handed over. + await expect(page).toHaveURL(/\/saml\/[^?]+\?.*loginClient=desktop/); + + credentialToken = new URL(page.url()).searchParams.get('saml_idp_credentialToken'); + expect(credentialToken).toBeTruthy(); + }); + + await test.step('expect the credential to be stored server side, ready for the app to redeem', async () => { + const storedCredential = await findCredentialToken(credentialToken as string); + + expect(storedCredential).not.toBeNull(); + expect(storedCredential?.userInfo?.profile).toBeDefined(); + expect(storedCredential?.userInfo?.profile?.email).toBe('samluser1@example.com'); + }); + + await test.step('expect the browser to remain unauthenticated', async () => { + await expect(poRegistration.btnLoginWithSaml).toBeVisible(); + + // Reload to prove no session was persisted for this browser. + await page.goto('/home'); + await expect(poRegistration.btnLoginWithSaml).toBeVisible(); + await expect(page.getByRole('button', { name: 'User menu' })).not.toBeVisible(); + }); + }); + }); + test.fixme('Data Sync - Custom Field Map', async () => { // Test the data sync using a custom fieldmap setting }); diff --git a/apps/meteor/tests/unit/server/lib/saml/server.tests.ts b/apps/meteor/tests/unit/server/lib/saml/server.tests.ts index 04c3a818f0afe..a2dc7f3bbf9a4 100644 --- a/apps/meteor/tests/unit/server/lib/saml/server.tests.ts +++ b/apps/meteor/tests/unit/server/lib/saml/server.tests.ts @@ -1131,6 +1131,36 @@ describe('SAML', () => { `saml/${credentialToken}?saml_idp_credentialToken=${credentialToken}`, ); }); + + describe('authorize RelayState encoding', () => { + it('should keep the provider when no loginClient is provided', () => { + expect(SAMLUtils.encodeAuthorizeRelayState('test-sp')).to.be.equal('test-sp'); + }); + + it('should ignore unsupported loginClient values and keep the provider', () => { + expect(SAMLUtils.encodeAuthorizeRelayState('test-sp', 'web')).to.be.equal('test-sp'); + expect(SAMLUtils.encodeAuthorizeRelayState('test-sp', '')).to.be.equal('test-sp'); + }); + + it('should round-trip provider and loginClient for supported clients', () => { + for (const loginClient of ['mobile', 'desktop']) { + const encoded = SAMLUtils.encodeAuthorizeRelayState('test-sp', loginClient); + expect(SAMLUtils.decodeAuthorizeRelayState(encoded)).to.be.deep.equal({ provider: 'test-sp', loginClient }); + } + }); + + it('should drop an unexpected loginClient echoed back in a compound RelayState', () => { + expect(SAMLUtils.decodeAuthorizeRelayState('provider=test-sp&loginClient=hacker')).to.be.deep.equal({ + provider: 'test-sp', + loginClient: undefined, + }); + }); + + it('should preserve providers containing URL-special characters', () => { + const encoded = SAMLUtils.encodeAuthorizeRelayState('a b&c=d', 'mobile'); + expect(SAMLUtils.decodeAuthorizeRelayState(encoded)).to.be.deep.equal({ provider: 'a b&c=d', loginClient: 'mobile' }); + }); + }); }); describe('[SAML.processRequest] validate action - assertion replay protection', () => { @@ -1185,6 +1215,7 @@ describe('SAML', () => { warn: sinon.stub(), log: sinon.stub(), getValidationActionRedirectPath: (token: string) => `_saml/validate/${token}`, + decodeAuthorizeRelayState: (provider: string) => ({ provider }), }, }, './getSAMLEnvelope': { getSAMLEnvelope: async () => ({ relayState: null }) }, diff --git a/packages/web-ui-registration/src/LoginServices.tsx b/packages/web-ui-registration/src/LoginServices.tsx index 8ac27f035cab2..cb5ce3ab9bb41 100644 --- a/packages/web-ui-registration/src/LoginServices.tsx +++ b/packages/web-ui-registration/src/LoginServices.tsx @@ -6,7 +6,7 @@ import { useTranslation } from 'react-i18next'; import type { LoginErrorState } from './LoginForm'; import LoginServicesButton from './LoginServicesButton'; -const servicesToBeShownOnDesktop = ['saml', 'cas', 'ldap']; +const servicesToBeShownOnDesktop = ['cas', 'ldap']; export type LoginServicesProps = { disabled?: boolean; setError: Dispatch> }; From 4bf6077f0d4ed5235ac4724627f29ab77edc476f Mon Sep 17 00:00:00 2001 From: gabriellsh <40830821+gabriellsh@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:25:57 +0000 Subject: [PATCH 3/4] test: `voice-calls-ee` flaky "hangup" locator matches "reject" after accepting call (#41741) Co-authored-by: Ashutosh Saxena <182192262+AlgoArtist06@users.noreply.github.com> --- .../e2e/page-objects/fragments/voice-calls.ts | 17 +++++++++++++---- apps/meteor/tests/e2e/voice-calls-ee.spec.ts | 18 +++++++++--------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/apps/meteor/tests/e2e/page-objects/fragments/voice-calls.ts b/apps/meteor/tests/e2e/page-objects/fragments/voice-calls.ts index 1570c0c972e58..d23e44b4b6eb6 100644 --- a/apps/meteor/tests/e2e/page-objects/fragments/voice-calls.ts +++ b/apps/meteor/tests/e2e/page-objects/fragments/voice-calls.ts @@ -25,8 +25,12 @@ export class VoiceCallControls { return this._controls.getByRole('button', { name: 'Accept', exact: true }); } + get reject(): Locator { + return this._controls.getByRole('button', { name: 'Reject', exact: true }); + } + get hangup(): Locator { - return this._controls.getByRole('button', { name: /End call|Reject/, exact: true }); + return this._controls.getByRole('button', { name: 'End call with', exact: false }); } get cancel(): Locator { @@ -150,11 +154,16 @@ export class Widget { await expect(this.callControls.hangup).toBeVisible(); } - async endCall(): Promise { + async hangup(): Promise { await this.callControls.hangup.click(); await expect(this.content).not.toBeVisible(); } + async reject(): Promise { + await this.callControls.reject.click(); + await expect(this.content).not.toBeVisible(); + } + async goToDm(): Promise { await this.headerControls.directMessage.click(); await expect(this.content).not.toBeVisible(); @@ -265,7 +274,7 @@ export class RoomSection { return timerToSeconds(text); } - async endCall(): Promise { + async hangup(): Promise { await this.callControls.hangup.click(); await expect(this.content).not.toBeVisible(); } @@ -329,7 +338,7 @@ export class PopoutPage extends RoomSection { this.page = page; } - override async endCall(): Promise { + override async hangup(): Promise { const pageClosed = new Promise((resolve) => this.page.on('close', () => resolve(true))); await this.callControls.hangup.click(); await expect(pageClosed).resolves.toBe(true); diff --git a/apps/meteor/tests/e2e/voice-calls-ee.spec.ts b/apps/meteor/tests/e2e/voice-calls-ee.spec.ts index ad93f25a37d42..2726cf8cc1788 100644 --- a/apps/meteor/tests/e2e/voice-calls-ee.spec.ts +++ b/apps/meteor/tests/e2e/voice-calls-ee.spec.ts @@ -50,7 +50,7 @@ test.describe('Internal Voice Calls - Enterprise Edition', () => { }); await test.step('user2 ends the call', async () => { - await user2.poHomeChannel.voiceCalls.widget.endCall(); + await user2.poHomeChannel.voiceCalls.widget.hangup(); await expect(user2.poHomeChannel.voiceCalls.widget.content).not.toBeVisible(); await expect(user1.poHomeChannel.voiceCalls.widget.content).not.toBeVisible(); }); @@ -91,7 +91,7 @@ test.describe('Internal Voice Calls - Enterprise Edition', () => { }); await test.step('should end the call from user1', async () => { - await user1.poHomeChannel.voiceCalls.widget.endCall(); + await user1.poHomeChannel.voiceCalls.widget.hangup(); await expect(user2.poHomeChannel.voiceCalls.widget.content).not.toBeVisible(); }); }); @@ -127,7 +127,7 @@ test.describe('Internal Voice Calls - Enterprise Edition', () => { }); await test.step('user3 ends the call', async () => { - await user3.poHomeChannel.voiceCalls.widget.endCall(); + await user3.poHomeChannel.voiceCalls.widget.hangup(); await expect(user3.poHomeChannel.voiceCalls.widget.content).not.toBeVisible(); await expect(user2.poHomeChannel.voiceCalls.widget.content).not.toBeVisible(); }); @@ -146,7 +146,7 @@ test.describe('Internal Voice Calls - Enterprise Edition', () => { }); await test.step('user2 declines the call', async () => { - await user2.poHomeChannel.voiceCalls.widget.endCall(); + await user2.poHomeChannel.voiceCalls.widget.reject(); }); await test.step('Verify call widget disappears', async () => { @@ -224,7 +224,7 @@ test.describe('Internal Voice Calls - In-room view - Enterprise Edition', () => }); await test.step('end call to clean up', async () => { - await user1.poHomeChannel.voiceCalls.roomSection.endCall(); + await user1.poHomeChannel.voiceCalls.roomSection.hangup(); await expect(user2.poHomeChannel.voiceCalls.widget.content).not.toBeVisible(); await expect(user2.poHomeChannel.voiceCalls.roomSection.content).not.toBeVisible(); }); @@ -269,7 +269,7 @@ test.describe('Internal Voice Calls - In-room view - Enterprise Edition', () => }); await test.step('end call to clean up', async () => { - await user1.poHomeChannel.voiceCalls.roomSection.endCall(); + await user1.poHomeChannel.voiceCalls.roomSection.hangup(); await expect(user2.poHomeChannel.voiceCalls.widget.content).not.toBeVisible(); }); }); @@ -312,7 +312,7 @@ test.describe('Internal Voice Calls - In-room view - Enterprise Edition', () => }); await test.step('end call to clean up', async () => { - await user1.poHomeChannel.voiceCalls.roomSection.endCall(); + await user1.poHomeChannel.voiceCalls.roomSection.hangup(); await expect(user2.poHomeChannel.voiceCalls.widget.content).not.toBeVisible(); await expect(user2.poHomeChannel.voiceCalls.roomSection.content).not.toBeVisible(); }); @@ -396,7 +396,7 @@ test.describe('Internal Voice Calls - Popout view - Enterprise Edition', () => { await user1.poHomeChannel.voiceCalls.openPopoutRoom(); await expect(user1.poHomeChannel.voiceCalls.popout.content).toBeVisible(); - await user1.poHomeChannel.voiceCalls.popout.endCall(); + await user1.poHomeChannel.voiceCalls.popout.hangup(); await expect(user2.poHomeChannel.voiceCalls.widget.content).not.toBeVisible(); await expect(user2.poHomeChannel.voiceCalls.roomSection.content).not.toBeVisible(); }); @@ -443,7 +443,7 @@ test.describe('Internal Voice Calls - Popout view - Enterprise Edition', () => { }); await test.step('end call with user2 from the popout', async () => { - await user2.poHomeChannel.voiceCalls.popout.endCall(); + await user2.poHomeChannel.voiceCalls.popout.hangup(); await expect(user1.poHomeChannel.voiceCalls.widget.content).not.toBeVisible(); await expect(user1.poHomeChannel.voiceCalls.roomSection.content).not.toBeVisible(); }); From c0a055092337672e875d5351a5cb43dd4abd8314 Mon Sep 17 00:00:00 2001 From: Pierre Lehnen <55164754+pierre-lehnen-rc@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:42:03 +0000 Subject: [PATCH 4/4] chore: mutate local SDPs, adding content tag to identify streams (#41654) --- packages/media-signaling/jest.config.ts | 6 + packages/media-signaling/package.json | 4 +- .../services/webrtc/IWebRTCProcessor.ts | 3 +- packages/media-signaling/src/lib/Call.ts | 34 +- .../src/lib/media/MediaStreamManager.ts | 6 +- .../src/lib/services/webrtc/Negotiation.ts | 29 +- .../src/lib/services/webrtc/Processor.ts | 50 +- .../src/lib/services/webrtc/index.ts | 1 + .../src/lib/services/webrtc/sdp.spec.ts | 496 ++++++++++++++++++ .../src/lib/services/webrtc/sdp.ts | 197 +++++++ 10 files changed, 791 insertions(+), 35 deletions(-) create mode 100644 packages/media-signaling/jest.config.ts create mode 100644 packages/media-signaling/src/lib/services/webrtc/sdp.spec.ts create mode 100644 packages/media-signaling/src/lib/services/webrtc/sdp.ts diff --git a/packages/media-signaling/jest.config.ts b/packages/media-signaling/jest.config.ts new file mode 100644 index 0000000000000..96c49597cc273 --- /dev/null +++ b/packages/media-signaling/jest.config.ts @@ -0,0 +1,6 @@ +import client from '@rocket.chat/jest-presets/client'; +import type { Config } from 'jest'; + +export default { + preset: client.preset, +} satisfies Config; diff --git a/packages/media-signaling/package.json b/packages/media-signaling/package.json index 56bc66b253c77..b2daac0c4212e 100644 --- a/packages/media-signaling/package.json +++ b/packages/media-signaling/package.json @@ -26,7 +26,9 @@ "dev": "tsc -p tsconfig.json --watch --preserveWatchOutput", "lint": "eslint .", "lint:fix": "eslint --fix .", - "test": "jest" + "test": "jest", + "testunit": "jest", + "typecheck": "tsc --noEmit" }, "dependencies": { "@rocket.chat/emitter": "workspace:~", diff --git a/packages/media-signaling/src/definition/services/webrtc/IWebRTCProcessor.ts b/packages/media-signaling/src/definition/services/webrtc/IWebRTCProcessor.ts index 7bd3dfcdd1939..8cb07a327822e 100644 --- a/packages/media-signaling/src/definition/services/webrtc/IWebRTCProcessor.ts +++ b/packages/media-signaling/src/definition/services/webrtc/IWebRTCProcessor.ts @@ -4,6 +4,7 @@ import type { IClientMediaCall } from '../../call'; import type { IMediaSignalLogger } from '../../logger'; import type { IMediaStreamManager } from '../../media/IMediaStreamManager'; import type { MediaStreamIdentification } from '../../media/MediaStreamIdentification'; +import type { ServerMediaSignalRemoteSDP } from '../../signals'; import type { IServiceProcessor, ServiceProcessorEvents } from '../IServiceProcessor'; export type WebRTCInternalStateMap = { @@ -48,7 +49,7 @@ export interface IWebRTCProcessor extends IServiceProcessor { - if (this.hidden || this.shouldIgnoreWebRTC()) { - return; - } - - this.config.logger?.debug('ClientMediaCall.processAnswerRequest', signal); - - this.requireWebRTC(); - - void this.negotiationManager.addNegotiation(signal.negotiationId, signal.sdp); - } - protected sendError(error: Partial): void { this.config.logger?.debug('ClientMediaCall.sendError', error); @@ -1080,30 +1068,22 @@ export class ClientMediaCall implements IClientMediaCall { } if (!this.isSignalTargetingThisSession(signal)) { - this.config.logger?.error('Received an offer request that is unsigned, or signed to a different session.'); + this.config.logger?.error('Received a remote sdp that is not signed to this session.'); return; } if (this.shouldIgnoreWebRTC()) { return; } + if (!['offer', 'answer'].includes(signal.sdp.type)) { + this.config.logger?.error('Unsupported remote sdp type.', signal.sdp.type); + return; + } this.requireWebRTC(); - if (signal.streams) { - this.webrtcProcessor.setRemoteIds(signal.streams); - } - switch (signal.sdp.type) { - case 'offer': - await this.processAnswerRequest(signal); - break; - case 'answer': - await this.negotiationManager.setRemoteDescription(signal.negotiationId, signal.sdp); - break; - default: - this.config.logger?.error('Unsupported sdp type.'); - return; - } + this.webrtcProcessor.setRemoteIds(signal); + await this.negotiationManager.setRemoteDescription(signal.negotiationId, signal.sdp); this.receivedRemoteSdp = true; this.updateClientState(); diff --git a/packages/media-signaling/src/lib/media/MediaStreamManager.ts b/packages/media-signaling/src/lib/media/MediaStreamManager.ts index 8925ff2b6929d..d29e8fea66938 100644 --- a/packages/media-signaling/src/lib/media/MediaStreamManager.ts +++ b/packages/media-signaling/src/lib/media/MediaStreamManager.ts @@ -101,9 +101,9 @@ export class MediaStreamManager implements IMediaStreamManager { return [this.mainRemote]; } - // A video track for an unidentified stream, let's ignore it - this.logger?.debug('unidentified stream, ignoring video track'); - return []; + // A video track for an unidentified stream - since the only video we support now is screen share, assume that's what this is + this.logger?.debug('unidentified stream, assuming screen-share'); + return [this.screenShareRemote]; } private createStream(remote: boolean, tag: string): MediaStreamWrapper { diff --git a/packages/media-signaling/src/lib/services/webrtc/Negotiation.ts b/packages/media-signaling/src/lib/services/webrtc/Negotiation.ts index 94bf864871fff..f6619660bb275 100644 --- a/packages/media-signaling/src/lib/services/webrtc/Negotiation.ts +++ b/packages/media-signaling/src/lib/services/webrtc/Negotiation.ts @@ -1,5 +1,6 @@ import { Emitter } from '@rocket.chat/emitter'; +import { SDP } from './sdp'; import type { IMediaSignalLogger, IWebRTCProcessor, NegotiationData, NegotiationEvents } from '../../../definition'; export class Negotiation { @@ -206,13 +207,39 @@ export class Negotiation { if (!sdp) { throw new Error('No local description'); } - return sdp; + return this.mutateLocalDescription(sdp); } catch (err) { this.logger?.error(err); this.fail('failed-to-get-local-description'); throw err; } } + + protected mutateLocalDescription(this: WebRTCNegotiation, description: RTCSessionDescriptionInit): RTCSessionDescriptionInit { + const { sdp, type } = description; + if (!sdp) { + return description; + } + + this.logger?.debug('MediaCallWebRTCProcessor.mutateLocalDescription', type); + + const mainStreamId = this.webrtcProcessor.streams.mainLocal.stream.id; + const screenShareStreamId = this.webrtcProcessor.streams.screenShareLocal.stream.id; + + const mutated = SDP.mutateSDPWithStreamContents(sdp, [ + { id: mainStreamId, content: 'main' }, + { id: screenShareStreamId, content: 'slides' }, + ]); + + if (sdp !== mutated) { + this.logger?.debug('SDP was mutated'); + } + + return { + type, + sdp: mutated, + }; + } } export abstract class WebRTCNegotiation extends Negotiation { diff --git a/packages/media-signaling/src/lib/services/webrtc/Processor.ts b/packages/media-signaling/src/lib/services/webrtc/Processor.ts index fda3466345952..fb1c13df9da59 100644 --- a/packages/media-signaling/src/lib/services/webrtc/Processor.ts +++ b/packages/media-signaling/src/lib/services/webrtc/Processor.ts @@ -1,8 +1,10 @@ import { Emitter } from '@rocket.chat/emitter'; +import { SDP } from './sdp'; import type { IWebRTCProcessor, WebRTCInternalStateMap, WebRTCProcessorConfig, WebRTCProcessorEvents } from '../../../definition'; import type { MediaStreamIdentification } from '../../../definition/media/MediaStreamIdentification'; import type { ServiceStateValue } from '../../../definition/services/IServiceProcessor'; +import type { ServerMediaSignalRemoteSDP } from '../../../definition/signals'; import { MediaStreamManager } from '../../media/MediaStreamManager'; import { getExternalWaiter, type PromiseWaiterData } from '../../utils/getExternalWaiter'; @@ -308,8 +310,52 @@ export class MediaCallWebRTCProcessor implements IWebRTCProcessor { await iceGatheringData.promise; } - public setRemoteIds(streams: MediaStreamIdentification[]): void { - this.streams.setRemoteIds(streams); + public setRemoteIds(signal: ServerMediaSignalRemoteSDP): void { + const { + streams, + sdp: { sdp }, + } = signal; + + const streamsFromSDP = sdp ? this.getRemoteIdsFromSDP(sdp) : []; + const allStreams = this.combineRemoteIds(streams || [], streamsFromSDP); + + if (allStreams.length) { + this.streams.setRemoteIds(allStreams); + } + } + + protected combineRemoteIds(streams1: MediaStreamIdentification[], streams2: MediaStreamIdentification[]): MediaStreamIdentification[] { + if (!streams2.length) { + return streams1; + } + if (!streams1.length) { + return streams2; + } + + const result = [...streams1]; + for (const stream of streams2) { + if (result.find(({ id }) => id === stream.id)) { + continue; + } + + result.push(stream); + } + + return result; + } + + protected getRemoteIdsFromSDP(sdp: string): MediaStreamIdentification[] { + const contentMap = SDP.getStreamContentMapFromSDP(sdp); + return Object.entries(contentMap) + .map(([id, content]) => { + const tag = SDP.getStreamTagByMediaContent(content); + if (!tag) { + return null; + } + + return { id, tag }; + }) + .filter((stream): stream is MediaStreamIdentification => Boolean(stream)); } public getLocalStreamIds(): MediaStreamIdentification[] { diff --git a/packages/media-signaling/src/lib/services/webrtc/index.ts b/packages/media-signaling/src/lib/services/webrtc/index.ts index c0b1f5e865bd7..162d8fb909014 100644 --- a/packages/media-signaling/src/lib/services/webrtc/index.ts +++ b/packages/media-signaling/src/lib/services/webrtc/index.ts @@ -1 +1,2 @@ export * from './Processor'; +export * from './sdp'; diff --git a/packages/media-signaling/src/lib/services/webrtc/sdp.spec.ts b/packages/media-signaling/src/lib/services/webrtc/sdp.spec.ts new file mode 100644 index 0000000000000..0250c19bf4cb6 --- /dev/null +++ b/packages/media-signaling/src/lib/services/webrtc/sdp.spec.ts @@ -0,0 +1,496 @@ +import { MediaDescription, SDP } from './sdp'; + +const CRLF = '\r\n'; + +const audioMediaLines = ['m=audio 9 UDP/TLS/RTP/SAVPF 111', 'c=IN IP4 0.0.0.0', 'a=mid:0', 'a=msid:audio-stream audio-track', 'a=sendrecv']; + +const videoMediaLines = [ + 'm=video 9 UDP/TLS/RTP/SAVPF 96', + 'c=IN IP4 0.0.0.0', + 'a=mid:1', + 'a=msid:video-stream video-track', + 'a=content:slides', + 'a=sendrecv', +]; + +const headerLines = ['v=0', 'o=- 4611731400430051336 2 IN IP4 127.0.0.1', 's=-', 't=0 0', 'a=group:BUNDLE 0 1']; + +const buildSDP = (lines: string[], delimiter = CRLF, trailing = true): string => { + const body = lines.join(delimiter); + return trailing ? `${body}${delimiter}` : body; +}; + +const sampleSDP = buildSDP([...headerLines, ...audioMediaLines, ...videoMediaLines]); + +// A realistic screen-share negotiation with what is currently supported: an +// audio-only `main` stream and a `slides` screen-share video stream, both +// content tags already present. +const screenShareMediaLines = [ + ...headerLines, + 'm=audio 9 UDP/TLS/RTP/SAVPF 111', + 'a=mid:0', + 'a=msid:main-stream audio-track', + 'a=content:main', + 'a=sendrecv', + 'm=video 9 UDP/TLS/RTP/SAVPF 96', + 'a=mid:1', + 'a=msid:screen-stream screen-track', + 'a=content:slides', + 'a=sendrecv', +]; + +// Same negotiation but with a camera video track on the `main` stream. +const screenShareWithCameraMediaLines = [ + ...headerLines, + 'm=video 9 UDP/TLS/RTP/SAVPF 96', + 'a=mid:0', + 'a=msid:main-stream camera-track', + 'a=content:main', + 'a=sendrecv', + 'm=video 9 UDP/TLS/RTP/SAVPF 96', + 'a=mid:1', + 'a=msid:screen-stream screen-track', + 'a=content:slides', + 'a=sendrecv', +]; + +// A full browser-generated offer as it arrives before any tagging: an audio +// `main` stream and a video screen-share stream, neither carrying an +// `a=content:` line yet. This is the common input to the mutation function. +const untaggedOfferLines = [ + 'v=0', + 'o=- 4611731400430051336 2 IN IP4 127.0.0.1', + 's=-', + 't=0 0', + 'a=group:BUNDLE 0 1', + 'a=extmap-allow-mixed', + 'a=msid-semantic: WMS main-stream screen-stream', + 'm=audio 9 UDP/TLS/RTP/SAVPF 111 63', + 'c=IN IP4 0.0.0.0', + 'a=rtcp:9 IN IP4 0.0.0.0', + 'a=ice-ufrag:4ZcD', + 'a=ice-pwd:by2Xr5jL6i2S3NqZ0P0m0Xa8', + 'a=fingerprint:sha-256 8F:32:1A:0B:44:9C:2E:11:7D:6F:AB:CD:EF:01:23:45:67:89:AB:CD:EF:01:23:45:67:89:AB:CD:EF:01:23:45', + 'a=setup:actpass', + 'a=mid:0', + 'a=sendrecv', + 'a=msid:main-stream audio-track', + 'a=rtcp-mux', + 'a=rtpmap:111 opus/48000/2', + 'm=video 9 UDP/TLS/RTP/SAVPF 96 97', + 'c=IN IP4 0.0.0.0', + 'a=rtcp:9 IN IP4 0.0.0.0', + 'a=ice-ufrag:4ZcD', + 'a=ice-pwd:by2Xr5jL6i2S3NqZ0P0m0Xa8', + 'a=fingerprint:sha-256 8F:32:1A:0B:44:9C:2E:11:7D:6F:AB:CD:EF:01:23:45:67:89:AB:CD:EF:01:23:45:67:89:AB:CD:EF:01:23:45', + 'a=setup:actpass', + 'a=mid:1', + 'a=sendrecv', + 'a=msid:screen-stream screen-track', + 'a=rtcp-mux', + 'a=rtpmap:96 VP8/90000', +]; + +describe('MediaDescription', () => { + describe('media type parsing', () => { + it('should parse the media type from the m= line', () => { + expect(new MediaDescription(['m=audio 9 UDP/TLS/RTP/SAVPF 111']).type).toBe('audio'); + expect(new MediaDescription(['m=video 9 UDP/TLS/RTP/SAVPF 96']).type).toBe('video'); + expect(new MediaDescription(['m=application 9 UDP/DTLS/SCTP webrtc-datachannel']).type).toBe('application'); + }); + + it('should return null when there is no m= line', () => { + expect(new MediaDescription(['a=mid:0', 'a=sendrecv']).type).toBeNull(); + }); + + it('should only use the first m= line to determine the type', () => { + const media = new MediaDescription(['m=audio 9 UDP/TLS/RTP/SAVPF 111', 'm=video 9 UDP/TLS/RTP/SAVPF 96']); + expect(media.type).toBe('audio'); + }); + }); + + describe('stream id parsing', () => { + it('should parse a stream id from the a=msid line', () => { + const media = new MediaDescription(['m=audio 9 UDP/TLS/RTP/SAVPF 111', 'a=msid:audio-stream audio-track']); + expect(media.streamIds).toEqual(['audio-stream']); + }); + + it('should parse a stream id even without a track id', () => { + const media = new MediaDescription(['a=msid:audio-stream']); + expect(media.streamIds).toEqual(['audio-stream']); + }); + + it('should ignore the "-" placeholder stream id', () => { + const media = new MediaDescription(['a=msid:- audio-track']); + expect(media.streamIds).toEqual([]); + }); + + it('should ignore an empty stream id', () => { + const media = new MediaDescription(['a=msid: audio-track']); + expect(media.streamIds).toEqual([]); + }); + + it('should deduplicate repeated stream ids', () => { + const media = new MediaDescription(['a=msid:stream track-a', 'a=msid:stream track-b']); + expect(media.streamIds).toEqual(['stream']); + }); + + it('should collect multiple distinct stream ids', () => { + const media = new MediaDescription(['a=msid:stream-a track-a', 'a=msid:stream-b track-b']); + expect(media.streamIds).toEqual(['stream-a', 'stream-b']); + }); + + it('should return an empty array when there is no msid line', () => { + const media = new MediaDescription(['m=audio 9 UDP/TLS/RTP/SAVPF 111']); + expect(media.streamIds).toEqual([]); + }); + }); + + describe('content parsing', () => { + it('should parse the content tag from the a=content line', () => { + const media = new MediaDescription(['m=video 9 UDP/TLS/RTP/SAVPF 96', 'a=content:slides']); + expect(media.content).toBe('slides'); + }); + + it('should return null when there is no content line', () => { + const media = new MediaDescription(['m=video 9 UDP/TLS/RTP/SAVPF 96']); + expect(media.content).toBeNull(); + }); + + it('should return null for an empty content value', () => { + const media = new MediaDescription(['a=content:']); + expect(media.content).toBeNull(); + }); + }); + + describe('lines', () => { + it('should expose a copy of the given lines', () => { + const input = ['m=audio 9 UDP/TLS/RTP/SAVPF 111', 'a=sendrecv']; + const media = new MediaDescription(input); + + expect(media.lines).toEqual(input); + expect(media.lines).not.toBe(input); + }); + + it('should not be affected by mutations to the original array', () => { + const input = ['m=audio 9 UDP/TLS/RTP/SAVPF 111']; + const media = new MediaDescription(input); + input.push('a=sendrecv'); + + expect(media.lines).toEqual(['m=audio 9 UDP/TLS/RTP/SAVPF 111']); + }); + }); + + describe('setContent', () => { + it('should add a content line when none exists', () => { + const media = new MediaDescription(['m=video 9 UDP/TLS/RTP/SAVPF 96', 'a=sendrecv']); + media.setContent('slides'); + + expect(media.content).toBe('slides'); + expect(media.lines).toEqual(['m=video 9 UDP/TLS/RTP/SAVPF 96', 'a=sendrecv', 'a=content:slides']); + }); + + it('should replace an existing content line', () => { + const media = new MediaDescription(['m=video 9 UDP/TLS/RTP/SAVPF 96', 'a=content:slides', 'a=sendrecv']); + media.setContent('main'); + + expect(media.content).toBe('main'); + expect(media.lines).toEqual(['m=video 9 UDP/TLS/RTP/SAVPF 96', 'a=sendrecv', 'a=content:main']); + expect(media.lines.filter((line) => line.startsWith('a=content:'))).toHaveLength(1); + }); + + it('should remove the content line when set to null', () => { + const media = new MediaDescription(['m=video 9 UDP/TLS/RTP/SAVPF 96', 'a=content:slides', 'a=sendrecv']); + media.setContent(null); + + expect(media.content).toBeNull(); + expect(media.lines).toEqual(['m=video 9 UDP/TLS/RTP/SAVPF 96', 'a=sendrecv']); + }); + + it('should be a no-op when the value does not change', () => { + const media = new MediaDescription(['m=video 9 UDP/TLS/RTP/SAVPF 96', 'a=content:slides', 'a=sendrecv']); + const linesBefore = media.lines; + media.setContent('slides'); + + expect(media.lines).toBe(linesBefore); + expect(media.content).toBe('slides'); + }); + }); +}); + +describe('SDP', () => { + describe('parsing', () => { + it('should split header lines from media descriptions', () => { + const sdp = new SDP(sampleSDP); + expect(sdp.medias).toHaveLength(2); + expect(sdp.medias[0].type).toBe('audio'); + expect(sdp.medias[1].type).toBe('video'); + }); + + it('should assign each media its own set of lines', () => { + const sdp = new SDP(sampleSDP); + expect(sdp.medias[0].lines[0]).toBe('m=audio 9 UDP/TLS/RTP/SAVPF 111'); + expect(sdp.medias[1].lines[0]).toBe('m=video 9 UDP/TLS/RTP/SAVPF 96'); + }); + + it('should parse stream ids and content per media', () => { + const sdp = new SDP(sampleSDP); + expect(sdp.medias[0].streamIds).toEqual(['audio-stream']); + expect(sdp.medias[0].content).toBeNull(); + expect(sdp.medias[1].streamIds).toEqual(['video-stream']); + expect(sdp.medias[1].content).toBe('slides'); + }); + + it('should handle SDPs delimited with \\n only', () => { + const sdp = new SDP(buildSDP([...headerLines, ...audioMediaLines, ...videoMediaLines], '\n')); + expect(sdp.medias).toHaveLength(2); + expect(sdp.medias[0].type).toBe('audio'); + }); + + it('should produce no media descriptions when there are no m= lines', () => { + const sdp = new SDP(buildSDP(headerLines)); + expect(sdp.medias).toHaveLength(0); + }); + + it('should handle an empty string', () => { + const sdp = new SDP(''); + expect(sdp.medias).toHaveLength(0); + }); + }); + + describe('joinLines', () => { + it('should round-trip an SDP delimited with CRLF', () => { + const sdp = new SDP(sampleSDP); + expect(sdp.joinLines()).toBe(sampleSDP); + }); + + it('should serialize using CRLF delimiters', () => { + const sdp = new SDP(buildSDP([...headerLines, ...audioMediaLines], '\n')); + const output = sdp.joinLines(); + expect(output).toContain(CRLF); + expect(output).toBe(buildSDP([...headerLines, ...audioMediaLines], CRLF)); + }); + + it('should not append a delimiter to empty lines', () => { + const sdp = new SDP(`${headerLines.join(CRLF)}${CRLF}`); + expect(sdp.joinLines()).not.toContain(`${CRLF}${CRLF}`); + }); + }); + + describe('setContentMediaByStreamId', () => { + it('should set the content on the media owning the stream id', () => { + const sdp = new SDP(sampleSDP); + sdp.setContentMediaByStreamId('audio-stream', 'main'); + + expect(sdp.medias[0].content).toBe('main'); + expect(sdp.medias[1].content).toBe('slides'); + }); + + it('should do nothing when no media owns the stream id', () => { + const sdp = new SDP(sampleSDP); + sdp.setContentMediaByStreamId('unknown-stream', 'main'); + + expect(sdp.medias[0].content).toBeNull(); + expect(sdp.medias[1].content).toBe('slides'); + }); + }); + + describe('mutateSDPWithStreamContents', () => { + it('should return the SDP unchanged when no streams are given', () => { + expect(SDP.mutateSDPWithStreamContents(sampleSDP, [])).toBe(sampleSDP); + }); + + it('should add content tags to the matching media', () => { + const output = SDP.mutateSDPWithStreamContents(sampleSDP, [{ id: 'audio-stream', content: 'main' }]); + expect(output).toContain(`a=content:main${CRLF}`); + expect(SDP.getStreamContentMapFromSDP(output)).toEqual({ + 'audio-stream': 'main', + 'video-stream': 'slides', + }); + }); + + it('should apply multiple stream contents at once', () => { + const output = SDP.mutateSDPWithStreamContents(sampleSDP, [ + { id: 'audio-stream', content: 'main' }, + { id: 'video-stream', content: 'speaker' }, + ]); + expect(SDP.getStreamContentMapFromSDP(output)).toEqual({ + 'audio-stream': 'main', + 'video-stream': 'speaker', + }); + }); + + it('should ignore stream ids that are not present', () => { + const output = SDP.mutateSDPWithStreamContents(sampleSDP, [{ id: 'ghost-stream', content: 'main' }]); + expect(SDP.getStreamContentMapFromSDP(output)).toEqual({ 'video-stream': 'slides' }); + }); + + describe('on a full offer with no content tags (common case)', () => { + it('should confirm the sample offer starts with no content tags', () => { + expect(buildSDP(untaggedOfferLines)).not.toContain('a=content:'); + expect(SDP.getStreamContentMapFromSDP(buildSDP(untaggedOfferLines))).toEqual({}); + }); + + it('should tag both streams and leave everything else intact', () => { + const input = buildSDP(untaggedOfferLines); + const output = SDP.mutateSDPWithStreamContents(input, [ + { id: 'main-stream', content: 'main' }, + { id: 'screen-stream', content: 'slides' }, + ]); + + expect(SDP.getStreamContentMapFromSDP(output)).toEqual({ + 'main-stream': 'main', + 'screen-stream': 'slides', + }); + // Exactly one tag added per media, none duplicated. + expect(output.match(/a=content:/g)).toHaveLength(2); + // Every original line survives the mutation, only content lines are new. + const addedLines = output.split(CRLF).filter((line) => line && !untaggedOfferLines.includes(line)); + expect(addedLines).toEqual(['a=content:main', 'a=content:slides']); + }); + + it('should append the content tag inside the media section that owns the stream', () => { + const output = SDP.mutateSDPWithStreamContents(buildSDP(untaggedOfferLines), [{ id: 'screen-stream', content: 'slides' }]); + const parsed = new SDP(output); + + expect(parsed.medias[0].type).toBe('audio'); + expect(parsed.medias[0].content).toBeNull(); + expect(parsed.medias[1].type).toBe('video'); + expect(parsed.medias[1].content).toBe('slides'); + // The tag lands in the video block, not the audio block or the header. + expect(parsed.medias[1].lines).toContain('a=content:slides'); + expect(parsed.medias[0].lines).not.toContain('a=content:slides'); + }); + + it('should only tag the requested stream and leave the other untagged', () => { + const output = SDP.mutateSDPWithStreamContents(buildSDP(untaggedOfferLines), [{ id: 'main-stream', content: 'main' }]); + expect(SDP.getStreamContentMapFromSDP(output)).toEqual({ 'main-stream': 'main' }); + expect(output.match(/a=content:/g)).toHaveLength(1); + }); + + it.each([ + ['CRLF', CRLF], + ['LF', '\n'], + ])('should tag a %s-delimited offer and serialize with CRLF', (_label, delimiter) => { + const output = SDP.mutateSDPWithStreamContents(buildSDP(untaggedOfferLines, delimiter), [ + { id: 'main-stream', content: 'main' }, + { id: 'screen-stream', content: 'slides' }, + ]); + + expect(output).toContain(`a=content:main${CRLF}`); + expect(output).toContain(`a=content:slides${CRLF}`); + expect(SDP.getStreamContentMapFromSDP(output)).toEqual({ + 'main-stream': 'main', + 'screen-stream': 'slides', + }); + }); + }); + }); + + describe('getStreamContentMapFromSDP', () => { + it('should map stream ids to their content tag', () => { + expect(SDP.getStreamContentMapFromSDP(sampleSDP)).toEqual({ 'video-stream': 'slides' }); + }); + + it('should skip media without a content tag', () => { + const sdp = buildSDP([...headerLines, ...audioMediaLines]); + expect(SDP.getStreamContentMapFromSDP(sdp)).toEqual({}); + }); + + it('should map every stream id of a media sharing the same content', () => { + const sdp = buildSDP([ + ...headerLines, + 'm=video 9 UDP/TLS/RTP/SAVPF 96', + 'a=msid:stream-a track-a', + 'a=msid:stream-b track-b', + 'a=content:slides', + ]); + expect(SDP.getStreamContentMapFromSDP(sdp)).toEqual({ + 'stream-a': 'slides', + 'stream-b': 'slides', + }); + }); + }); + + describe('getStreamTagByMediaContent', () => { + it('should map "slides" to "screen-share"', () => { + expect(SDP.getStreamTagByMediaContent('slides')).toBe('screen-share'); + }); + + it('should map "main" to "main"', () => { + expect(SDP.getStreamTagByMediaContent('main')).toBe('main'); + }); + + it.each(['speaker', 'sl', 'alt', 'unknown', ''])('should return null for %p', (content) => { + expect(SDP.getStreamTagByMediaContent(content)).toBeNull(); + }); + }); + + describe('main/slides screen-share SDP', () => { + it('should parse the audio-only main stream and the slides video stream', () => { + const sdp = new SDP(buildSDP(screenShareMediaLines)); + + expect(sdp.medias[0].type).toBe('audio'); + expect(sdp.medias[0].content).toBe('main'); + expect(sdp.medias[0].streamIds).toEqual(['main-stream']); + + expect(sdp.medias[1].type).toBe('video'); + expect(sdp.medias[1].content).toBe('slides'); + expect(sdp.medias[1].streamIds).toEqual(['screen-stream']); + }); + + it.each([ + ['CRLF', CRLF], + ['LF', '\n'], + ])('should read existing main and slides content tags from %s input', (_label, delimiter) => { + const sdp = buildSDP(screenShareMediaLines, delimiter); + expect(SDP.getStreamContentMapFromSDP(sdp)).toEqual({ + 'main-stream': 'main', + 'screen-stream': 'slides', + }); + }); + + it.each([ + ['CRLF', CRLF], + ['LF', '\n'], + ])('should preserve the exact content tags when round-tripping %s input', (_label, delimiter) => { + const sdp = new SDP(buildSDP(screenShareMediaLines, delimiter)); + const output = sdp.joinLines(); + expect(output).toContain(`a=content:main${CRLF}`); + expect(output).toContain(`a=content:slides${CRLF}`); + expect(output.match(/a=content:/g)).toHaveLength(2); + }); + + it('should replace an existing tag in place without leaving a stale one', () => { + const output = SDP.mutateSDPWithStreamContents(buildSDP(screenShareMediaLines), [{ id: 'main-stream', content: 'slides' }]); + + // The main section flips main -> slides; the screen-share section is untouched. + expect(SDP.getStreamContentMapFromSDP(output)).toEqual({ + 'main-stream': 'slides', + 'screen-stream': 'slides', + }); + // No stale `main` tag left behind after replacement. + expect(output).not.toContain('a=content:main'); + expect(output.match(/a=content:/g)).toHaveLength(2); + }); + + it('should place tags only on the media owning each stream id', () => { + const sdp = new SDP(buildSDP(screenShareMediaLines)); + sdp.setContentMediaByStreamId('screen-stream', 'speaker'); + + expect(sdp.medias[0].content).toBe('main'); + expect(sdp.medias[1].content).toBe('speaker'); + }); + + it('should handle a main stream carrying a camera video track', () => { + const sdp = new SDP(buildSDP(screenShareWithCameraMediaLines)); + + expect(sdp.medias[0].type).toBe('video'); + expect(sdp.medias[0].content).toBe('main'); + expect(SDP.getStreamContentMapFromSDP(buildSDP(screenShareWithCameraMediaLines))).toEqual({ + 'main-stream': 'main', + 'screen-stream': 'slides', + }); + }); + }); +}); diff --git a/packages/media-signaling/src/lib/services/webrtc/sdp.ts b/packages/media-signaling/src/lib/services/webrtc/sdp.ts new file mode 100644 index 0000000000000..c984fd17f2aa5 --- /dev/null +++ b/packages/media-signaling/src/lib/services/webrtc/sdp.ts @@ -0,0 +1,197 @@ +/* Content tags defined by RFC 4796 - external SDPs may still have other values */ +type MediaContent = 'slides' | 'speaker' | 'sl' | 'main' | 'alt'; +type StreamContent = { id: string; content: MediaContent }; + +const lineDelimiter = '\r\n'; + +export class MediaDescription { + private _lines: string[]; + + public readonly streamIds: string[]; + + private _type: string | null = null; + + private _content: string | null = null; + + public get type(): string | null { + return this._type; + } + + public get content(): string | null { + return this._content; + } + + public get lines(): string[] { + return this._lines; + } + + constructor(lines: string[]) { + this._lines = [...lines]; + this.streamIds = []; + this.parseLines(); + } + + private parseLines() { + for (const line of this.lines) { + this.parseMediaType(line); + this.parseStreamId(line); + this.parseContent(line); + } + } + + private parseMediaType(line: string) { + if (this._type || !line.startsWith('m=')) { + return; + } + + this._type = line.match(/^m=(\w+)/)?.[1] || null; + } + + private parseStreamId(line: string) { + if (!line.startsWith('a=msid:')) { + return; + } + + const streamId = line.slice('a=msid:'.length).split(' ')[0]; + if (!streamId || streamId === '-') { + return; + } + + if (this.streamIds.includes(streamId)) { + return; + } + + this.streamIds.push(streamId); + } + + private parseContent(line: string) { + if (!line.startsWith('a=content:')) { + return; + } + + this._content = line.replace('a=content:', '') || null; + } + + public setContent(value: MediaContent | null) { + if (this._content === value) { + return; + } + + this._content = value || null; + const lines = this.lines.filter((line) => !line.startsWith('a=content:')); + this._lines = [...lines, ...(value ? [`a=content:${value}`] : [])]; + } +} + +export class SDP { + private headerLines: string[]; + + public readonly medias: MediaDescription[]; + + constructor(sdp: string) { + this.headerLines = []; + this.medias = []; + + this.parseSDP(sdp); + } + + private addMediaDescription(lines?: string[]) { + if (!lines?.length) { + return; + } + + this.medias.push(new MediaDescription(lines)); + } + + private parseSDP(sdp: string) { + const allLines = sdp.split(/\r?\n/); + + let currentMediaLines: string[] | undefined; + + for (const line of allLines) { + if (line.startsWith('m=')) { + this.addMediaDescription(currentMediaLines); + + currentMediaLines = [line]; + continue; + } + + if (!currentMediaLines) { + this.headerLines.push(line); + continue; + } + + currentMediaLines.push(line); + } + + this.addMediaDescription(currentMediaLines); + } + + public joinLines(): string { + const lines = [...this.headerLines, ...this.medias.flatMap(({ lines }) => lines)]; + + const delimitedLines = lines.map((line) => { + if (!line) { + return line; + } + + return `${line}${lineDelimiter}`; + }); + + return delimitedLines.join(''); + } + + public setContentMediaByStreamId(streamId: string, content: MediaContent) { + for (const media of this.medias) { + if (media.streamIds.includes(streamId)) { + media.setContent(content); + } + } + } + + public static mutateSDPWithStreamContents(sdp: string, streams: StreamContent[]): string { + if (!streams.length) { + return sdp; + } + + const parsed = new SDP(sdp); + + for (const { id, content } of streams) { + parsed.setContentMediaByStreamId(id, content); + } + + return parsed.joinLines(); + } + + /* + * Returns an object where the key is a stream id and the object is a stream content tag + */ + public static getStreamContentMapFromSDP(sdp: string): Record { + const streams: Record = {}; + + const parsed = new SDP(sdp); + for (const media of parsed.medias) { + const { streamIds, content } = media; + if (!streamIds.length || !content) { + continue; + } + + for (const id of streamIds) { + streams[id] = content; + } + } + + return streams; + } + + public static getStreamTagByMediaContent(content: string): string | null { + switch (content) { + case 'slides': + return 'screen-share'; + case 'main': + return 'main'; + default: + return null; + } + } +}