From 295f5caea74404949b52858471a04c574e47cc97 Mon Sep 17 00:00:00 2001 From: Romaric Mourgues Date: Wed, 4 Jan 2023 18:36:53 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=20Security=20update=20(#2678)=20(#?= =?UTF-8?q?2681)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix https://huntr.dev/bounties/bfd935f4-2d1d-4d3f-8b59-522abe7dd065/ * Fix access control over posting messages to channels / threads * Fix typo * Fix some tests * Fix one of the tests * Fix test * Fix another test * Still fixing the search one * Fix 2 tests cases * Fixed some stuff * Fixed some stuff * Finished fixing tests --- .../messages/web/controllers/messages.ts | 32 +++++- .../messages/web/controllers/threads.ts | 22 ++++ .../messages/web/controllers/views.ts | 18 ++- .../e2e/messages/messages.messages.spec.ts | 105 ++---------------- .../test/e2e/messages/messages.search.spec.ts | 29 +++-- .../messages/messages.views.channel.spec.ts | 24 +++- twake/backend/node/test/e2e/messages/utils.ts | 35 +++++- twake/frontend/src/app/atoms/link/index.tsx | 2 +- .../twacode/pseudo-markdown-dictionary.tsx | 6 +- .../messages/message/parts/Blocks.tsx | 2 +- .../messages/message/parts/LinkPreview.tsx | 2 +- .../messages/message/parts/MessageHeader.tsx | 2 +- .../app/views/integration/components/apps.js | 9 +- 13 files changed, 162 insertions(+), 126 deletions(-) diff --git a/twake/backend/node/src/services/messages/web/controllers/messages.ts b/twake/backend/node/src/services/messages/web/controllers/messages.ts index 3f92a4ea71..b29b3a1075 100644 --- a/twake/backend/node/src/services/messages/web/controllers/messages.ts +++ b/twake/backend/node/src/services/messages/web/controllers/messages.ts @@ -14,7 +14,7 @@ import { ThreadExecutionContext, } from "../../types"; import { handleError } from "../../../../utils/handleError"; -import { Pagination } from "../../../../core/platform/framework/api/crud-service"; +import { CrudException, Pagination } from "../../../../core/platform/framework/api/crud-service"; import { getThreadMessageWebsocketRoom } from "../realtime"; import { ThreadPrimaryKey } from "../../entities/threads"; import { extendExecutionContentWithChannel } from "./index"; @@ -74,6 +74,36 @@ export class MessagesController throw "Message must be in a thread"; } + let hasOneMembership = false; + for (const participant of thread.participants) { + if (thread.created_by === context.user.id) { + hasOneMembership = true; + break; + } + if (participant.type === "channel") { + const isMember = await gr.services.channels.members.getChannelMember( + { id: context.user.id }, + { + company_id: participant.company_id, + workspace_id: participant.workspace_id, + id: participant.id, + }, + ); + if (isMember) { + hasOneMembership = true; + break; + } + } else if (participant.type === "user") { + if (participant.id === context.user.id) { + hasOneMembership = true; + break; + } + } + } + if (!hasOneMembership) { + throw CrudException.notFound("You can't post in this thread"); + } + const result = await gr.services.messages.messages.save( { id: request.params.message_id || undefined, diff --git a/twake/backend/node/src/services/messages/web/controllers/threads.ts b/twake/backend/node/src/services/messages/web/controllers/threads.ts index cd66e02770..495179b1d1 100644 --- a/twake/backend/node/src/services/messages/web/controllers/threads.ts +++ b/twake/backend/node/src/services/messages/web/controllers/threads.ts @@ -10,6 +10,7 @@ import { handleError } from "../../../../utils/handleError"; import { CompanyExecutionContext } from "../../types"; import { ParticipantObject, Thread } from "../../entities/threads"; import gr from "../../../global-resolver"; +import { CrudException } from "../../../../core/platform/framework/api/crud-service"; export class ThreadsController implements @@ -39,6 +40,27 @@ export class ThreadsController reply: FastifyReply, ): Promise> { const context = getCompanyExecutionContext(request); + + const participants = + (request.body.resource.participants?.length + ? request.body.resource?.participants + : request.body.options?.participants?.add) || []; + for (const participant of participants) { + if (participant.type === "channel") { + const isMember = await gr.services.channels.members.getChannelMember( + { id: context.user.id }, + { + company_id: participant.company_id, + workspace_id: participant.workspace_id, + id: participant.id, + }, + ); + if (!isMember) { + throw CrudException.notFound("Channel not found"); + } + } + } + try { const result = await gr.services.messages.threads.save( { diff --git a/twake/backend/node/src/services/messages/web/controllers/views.ts b/twake/backend/node/src/services/messages/web/controllers/views.ts index de1f641e9f..4cd20aacac 100644 --- a/twake/backend/node/src/services/messages/web/controllers/views.ts +++ b/twake/backend/node/src/services/messages/web/controllers/views.ts @@ -2,7 +2,11 @@ import { FastifyReply, FastifyRequest } from "fastify"; import { ResourceListResponse } from "../../../../utils/types"; import { Message } from "../../entities/messages"; import { handleError } from "../../../../utils/handleError"; -import { ListResult, Pagination } from "../../../../core/platform/framework/api/crud-service"; +import { + CrudException, + ListResult, + Pagination, +} from "../../../../core/platform/framework/api/crud-service"; import { ChannelViewExecutionContext, FlatFileFromMessage, @@ -39,6 +43,18 @@ export class ViewsController { const query = { ...request.query, include_users: request.query.include_users }; const context = getChannelViewExecutionContext(request); + const isMember = await gr.services.channels.members.getChannelMember( + { id: context.user.id }, + { + company_id: request.params.company_id, + workspace_id: request.params.workspace_id, + id: request.params.channel_id, + }, + ); + if (!isMember) { + throw CrudException.notFound("Channel not found"); + } + let resources: ListResult; try { diff --git a/twake/backend/node/test/e2e/messages/messages.messages.spec.ts b/twake/backend/node/test/e2e/messages/messages.messages.spec.ts index 0eec53a0de..2d30b256c1 100644 --- a/twake/backend/node/test/e2e/messages/messages.messages.spec.ts +++ b/twake/backend/node/test/e2e/messages/messages.messages.spec.ts @@ -9,7 +9,13 @@ import { } from "../../../src/utils/types"; import { deserialize } from "class-transformer"; import { ParticipantObject, Thread } from "../../../src/services/messages/entities/threads"; -import { createMessage, e2e_createMessage, e2e_createThread } from "./utils"; +import { + createMessage, + createParticipant, + e2e_createChannel, + e2e_createMessage, + e2e_createThread, +} from "./utils"; import { Message } from "../../../src/services/messages/entities/messages"; import { v1 as uuidv1 } from "uuid"; import { MessageWithReplies } from "../../../src/services/messages/types"; @@ -98,98 +104,10 @@ describe("The Messages feature", () => { expect(listResponse.statusCode).toBe(200); expect(listResult.resources.length).toBe(3); }); - - it("should move the message in threads", async () => { - const userA = uuidv1(); - const userB = uuidv1(); - const userC = uuidv1(); - - const thread1Request = await e2e_createThread( - platform, - [], - createMessage({ text: "Message A in thread 1", user_id: userA }), - ); - const thread1Result: ResourceUpdateResponse = deserialize( - ResourceUpdateResponse, - thread1Request.body, - ); - const thread1: Thread = thread1Result.resource; - - const thread2Request = await e2e_createThread( - platform, - [], - createMessage({ text: "Message B in thread 2", user_id: userB }), - ); - const thread2Result: ResourceUpdateResponse = deserialize( - ResourceUpdateResponse, - thread2Request.body, - ); - const thread2: Thread = thread2Result.resource; - - const messageCRequest = await e2e_createMessage( - platform, - thread1.id, - createMessage({ text: "Message C in thread 1", user_id: userC }), - ); - const messageCResult: ResourceUpdateResponse = deserialize( - ResourceUpdateResponse, - messageCRequest.body, - ); - const messageC: Message = messageCResult.resource; - - const messageCAfterMoveRequest = await platform.app.inject({ - method: "POST", - url: `${url}/companies/${platform.workspace.company_id}/threads/${thread2.id}/messages/${messageC.id}`, - headers: { - authorization: `Bearer ${await platform.auth.getJWTToken({ sub: userA })}`, - }, - payload: { - resource: messageC, - options: { - previous_thread: thread1.id, - }, - }, - }); - const messageCAfterMoveResult: ResourceUpdateResponse = deserialize( - ResourceUpdateResponse, - messageCAfterMoveRequest.body, - ); - const messageCAfterMove: Message = messageCAfterMoveResult.resource; - - //See if message was moved correctly to the new thread - expect(messageCAfterMove.user_id).toBe(userC); - expect(messageCAfterMove.thread_id).toBe(thread2.id); - - const messageCAfterMove2Request = await platform.app.inject({ - method: "POST", - url: `${url}/companies/${platform.workspace.company_id}/threads/${messageC.id}/messages/${messageC.id}`, - headers: { - authorization: `Bearer ${await platform.auth.getJWTToken({ sub: userA })}`, - }, - payload: { - resource: messageC, - options: { - previous_thread: thread2.id, - }, - }, - }); - const messageCAfterMove2Result: ResourceUpdateResponse = deserialize( - ResourceUpdateResponse, - messageCAfterMove2Request.body, - ); - const messageCAfter2Move: Message = messageCAfterMove2Result.resource; - - //See if message was moved correctly to new standalone thread - expect(messageCAfter2Move.user_id).toBe(userC); - expect(messageCAfter2Move.thread_id).toBe(messageCAfter2Move.id); - - //TODO check counts - }); }); describe("Inbox", () => { it("Should get recent user messages", async done => { - const channel = channelUtils.getChannel(); const directChannelIn = channelUtils.getDirectChannel(); const members = [platform.currentUser.id, uuidv1()]; const directWorkspace: Workspace = { @@ -198,7 +116,6 @@ describe("The Messages feature", () => { }; await Promise.all([ - gr.services.channels.channels.save(channel, {}, getContext()), gr.services.channels.channels.save( directChannelIn, { @@ -209,12 +126,12 @@ describe("The Messages feature", () => { ]); const recipient: ParticipantObject = { - company_id: platform.workspace.company_id, created_at: 0, created_by: "", - id: channel.id, type: "channel", + company_id: directChannelIn.company_id, workspace_id: "direct", + id: directChannelIn.id, }; for (let i = 0; i < 6; i++) { @@ -270,8 +187,8 @@ describe("The Messages feature", () => { application_id: null, text: expect.any(String), cache: { - company_id: channel.company_id, - channel_id: channel.id, + company_id: directChannelIn.company_id, + channel_id: directChannelIn.id, }, }); } diff --git a/twake/backend/node/test/e2e/messages/messages.search.spec.ts b/twake/backend/node/test/e2e/messages/messages.search.spec.ts index 406300eab0..f1c3afdef2 100644 --- a/twake/backend/node/test/e2e/messages/messages.search.spec.ts +++ b/twake/backend/node/test/e2e/messages/messages.search.spec.ts @@ -2,7 +2,7 @@ import { afterAll, beforeAll, describe, expect, it } from "@jest/globals"; import { init, TestPlatform } from "../setup"; import { TestDbService } from "../utils.prepare.db"; import { v1 as uuidv1 } from "uuid"; -import { createMessage, e2e_createMessage, e2e_createThread } from "./utils"; +import { createMessage, e2e_createChannel, e2e_createMessage, e2e_createThread } from "./utils"; import { ResourceUpdateResponse } from "../../../src/utils/types"; import { ParticipantObject, Thread } from "../../../src/services/messages/entities/threads"; import { deserialize } from "class-transformer"; @@ -121,22 +121,22 @@ describe("The /messages API", () => { done(); }); it("Filter out messages from channels we are not member of", async done => { - const channel = await createChannel(); - const anotherChannel = await createChannel(uuidv1()); const anotherUserId = uuidv1(); + const channel = await e2e_createChannel(platform, [platform.currentUser.id, anotherUserId]); + const anotherChannel = await e2e_createChannel(platform, [anotherUserId], anotherUserId); //Test user is not the owner const participant = { type: "channel", - id: channel.id, - company_id: platform.workspace.company_id, - workspace_id: platform.workspace.workspace_id, + id: channel.resource.id, + company_id: channel.resource.company_id, + workspace_id: channel.resource.workspace_id, } as ParticipantObject; const participant2 = { type: "channel", - id: anotherChannel.id, - company_id: platform.workspace.company_id, - workspace_id: platform.workspace.workspace_id, + id: anotherChannel.resource.id, + company_id: anotherChannel.resource.company_id, + workspace_id: anotherChannel.resource.workspace_id, } as ParticipantObject; const file = new MessageFile(); @@ -148,7 +148,7 @@ describe("The /messages API", () => { await createReply(firstThreadId, "Filtered message 1-3"); await createReply(firstThreadId, "Filtered message 1-4", { files: [file] }); - const secondThreadId = await createThread("Filtered thread 2", [participant2]); + const secondThreadId = await createThread("Filtered thread 2", [participant2], anotherUserId); await createReply(secondThreadId, "Filtered message 2-1"); await createReply(secondThreadId, "Filtered message 2-2"); await createReply(secondThreadId, "Filtered message 2-3"); @@ -204,8 +204,13 @@ describe("The /messages API", () => { return creationResult.entity; } - async function createThread(text, participants: ParticipantObject[]) { - const response = await e2e_createThread(platform, participants, createMessage({ text: text })); + async function createThread(text, participants: ParticipantObject[], owner?: string) { + const response = await e2e_createThread( + platform, + participants, + createMessage({ text: text, user_id: owner }), + owner, + ); const result: ResourceUpdateResponse = deserialize( ResourceUpdateResponse, diff --git a/twake/backend/node/test/e2e/messages/messages.views.channel.spec.ts b/twake/backend/node/test/e2e/messages/messages.views.channel.spec.ts index 615db91d8f..14b0029870 100644 --- a/twake/backend/node/test/e2e/messages/messages.views.channel.spec.ts +++ b/twake/backend/node/test/e2e/messages/messages.views.channel.spec.ts @@ -5,7 +5,13 @@ import { ResourceListResponse, ResourceUpdateResponse } from "../../../src/utils import { deserialize } from "class-transformer"; import { v4 as uuidv4 } from "uuid"; import { Thread } from "../../../src/services/messages/entities/threads"; -import { createMessage, createParticipant, e2e_createMessage, e2e_createThread } from "./utils"; +import { + createMessage, + createParticipant, + e2e_createChannel, + e2e_createMessage, + e2e_createThread, +} from "./utils"; import { MessageWithReplies } from "../../../src/services/messages/types"; describe("The Messages feature", () => { @@ -42,7 +48,7 @@ describe("The Messages feature", () => { describe("On user use messages in channel view", () => { it("should create a message and retrieve it in channel view", async () => { - const channelId = uuidv4(); + const channel = await e2e_createChannel(platform, [platform.currentUser.id]); const response = await e2e_createThread( platform, @@ -50,7 +56,9 @@ describe("The Messages feature", () => { createParticipant( { type: "channel", - id: channelId, + id: channel.resource.id, + workspace_id: channel.resource.workspace_id, + company_id: channel.resource.company_id, }, platform, ), @@ -73,7 +81,9 @@ describe("The Messages feature", () => { createParticipant( { type: "channel", - id: channelId, + id: channel.resource.id, + workspace_id: channel.resource.workspace_id, + company_id: channel.resource.company_id, }, platform, ), @@ -89,7 +99,9 @@ describe("The Messages feature", () => { createParticipant( { type: "channel", - id: channelId, + id: channel.resource.id, + workspace_id: channel.resource.workspace_id, + company_id: channel.resource.company_id, }, platform, ), @@ -100,7 +112,7 @@ describe("The Messages feature", () => { const jwtToken = await platform.auth.getJWTToken(); const listResponse = await platform.app.inject({ method: "GET", - url: `${url}/companies/${platform.workspace.company_id}/workspaces/${platform.workspace.workspace_id}/channels/${channelId}/feed?replies_per_thread=3&include_users=1`, + url: `${url}/companies/${channel.resource.company_id}/workspaces/${channel.resource.workspace_id}/channels/${channel.resource.id}/feed?replies_per_thread=3&include_users=1`, headers: { authorization: `Bearer ${jwtToken}`, }, diff --git a/twake/backend/node/test/e2e/messages/utils.ts b/twake/backend/node/test/e2e/messages/utils.ts index 6b813ca506..6a5380ec91 100644 --- a/twake/backend/node/test/e2e/messages/utils.ts +++ b/twake/backend/node/test/e2e/messages/utils.ts @@ -1,19 +1,50 @@ -import { v1 } from "uuid"; +import { deserialize } from "class-transformer"; import { getInstance as getMessageInstance, Message, } from "../../../src/services/messages/entities/messages"; import { ParticipantObject } from "../../../src/services/messages/entities/threads"; +import { Channel, ResourceCreateResponse } from "../../../src/utils/types"; import { TestPlatform } from "../setup"; const url = "/internal/services/messages/v1"; +export const e2e_createChannel = async ( + platform: TestPlatform, + members: string[], + owner?: string, +) => { + const jwtToken = await platform.auth.getJWTToken({ sub: owner || platform.currentUser.id }); + const response = await platform.app.inject({ + method: "POST", + url: `/internal/services/channels/v1/companies/${platform.workspace.company_id}/workspaces/direct/channels`, + headers: { + authorization: `Bearer ${jwtToken}`, + }, + payload: { + options: { + members, + }, + resource: { + description: "A direct channel description", + visibility: "direct", + }, + }, + }); + const channelCreateResult: ResourceCreateResponse = deserialize( + ResourceCreateResponse, + response.body, + ); + return channelCreateResult; +}; + export const e2e_createThread = async ( platform: TestPlatform, participants: ParticipantObject[], message: Message, + owner?: string, ) => { - const jwtToken = await platform.auth.getJWTToken(); + const jwtToken = await platform.auth.getJWTToken({ sub: owner || platform.currentUser.id }); const res = await platform.app.inject({ method: "POST", url: `${url}/companies/${platform.workspace.company_id}/threads`, diff --git a/twake/frontend/src/app/atoms/link/index.tsx b/twake/frontend/src/app/atoms/link/index.tsx index 6968f600c2..fee0bceb32 100644 --- a/twake/frontend/src/app/atoms/link/index.tsx +++ b/twake/frontend/src/app/atoms/link/index.tsx @@ -25,7 +25,7 @@ export default function A( return ( diff --git a/twake/frontend/src/app/components/twacode/pseudo-markdown-dictionary.tsx b/twake/frontend/src/app/components/twacode/pseudo-markdown-dictionary.tsx index db63a9f344..d92b617999 100644 --- a/twake/frontend/src/app/components/twacode/pseudo-markdown-dictionary.tsx +++ b/twake/frontend/src/app/components/twacode/pseudo-markdown-dictionary.tsx @@ -92,7 +92,7 @@ export const DynamicComponent = ({ } return ( // eslint-disable-next-line react/jsx-no-target-blank - + {child} ); @@ -334,9 +334,7 @@ class PseudoMarkdownDictionary { object: (_child: any, object: any) => , }, image: { - object: (_child: any, object: any) => ( - - ), + object: (_child: any, object: any) => , }, icon: { object: (_child: any, object: any) => , diff --git a/twake/frontend/src/app/views/applications/messages/message/parts/Blocks.tsx b/twake/frontend/src/app/views/applications/messages/message/parts/Blocks.tsx index 2c809108df..afcbd833a0 100644 --- a/twake/frontend/src/app/views/applications/messages/message/parts/Blocks.tsx +++ b/twake/frontend/src/app/views/applications/messages/message/parts/Blocks.tsx @@ -40,7 +40,7 @@ const Link = ({ href, children }: { href: string; children: string }) => { target = '_self'; } return ( - + {children} ); diff --git a/twake/frontend/src/app/views/applications/messages/message/parts/LinkPreview.tsx b/twake/frontend/src/app/views/applications/messages/message/parts/LinkPreview.tsx index 9ebef88eb0..96041d9504 100644 --- a/twake/frontend/src/app/views/applications/messages/message/parts/LinkPreview.tsx +++ b/twake/frontend/src/app/views/applications/messages/message/parts/LinkPreview.tsx @@ -34,7 +34,7 @@ export default ({ preview }: PropsType): React.ReactElement => {