From e1bab2e353a075ee049c3285992e8bc1319fc789 Mon Sep 17 00:00:00 2001 From: Hampus Date: Sun, 30 Aug 2026 22:15:49 +0200 Subject: [PATCH] perf(message): reuse channel auth across send and edit (#2135) --- .../services/message/MessageEditService.ts | 16 ++- .../services/message/MessageRequestService.ts | 6 +- .../message/MessageRetrievalService.ts | 11 ++- .../services/message/MessageSendService.ts | 18 +++- .../tests/MessageSendPermissions.test.ts | 98 ++++++++++++++++++- 5 files changed, 133 insertions(+), 16 deletions(-) diff --git a/fluxer_api/src/api/channel/services/message/MessageEditService.ts b/fluxer_api/src/api/channel/services/message/MessageEditService.ts index e933aa5a8..990678e22 100644 --- a/fluxer_api/src/api/channel/services/message/MessageEditService.ts +++ b/fluxer_api/src/api/channel/services/message/MessageEditService.ts @@ -18,6 +18,7 @@ import type {IUserRepository} from '../../../user/IUserRepository'; import {assertGuildMemberCanCommunicate} from '../../../utils/GuildCommunicationUtils'; import type {MessageUpdateRequest} from '../../MessageTypes'; import type {IChannelRepositoryAggregate} from '../../repositories/IChannelRepositoryAggregate'; +import type {AuthenticatedChannel} from '../AuthenticatedChannel'; import type {MessageChannelAuthService} from './MessageChannelAuthService'; import type {MessageDispatchService} from './MessageDispatchService'; import type {MessageEmbedAttachmentResolver} from './MessageEmbedAttachmentResolver'; @@ -32,6 +33,11 @@ const MESSAGE_LOCK_TTL_SECONDS = 5; const MESSAGE_LOCK_ACQUIRE_ATTEMPTS = 6; const MESSAGE_LOCK_RETRY_DELAY_MS = 50; +interface EditMessageResult { + message: Message; + authChannel: AuthenticatedChannel; +} + interface MessageEditServiceDeps { channelRepository: IChannelRepositoryAggregate; userRepository: IUserRepository; @@ -61,11 +67,12 @@ export class MessageEditService { messageId: MessageID; data: MessageUpdateRequest; requestCache: RequestCache; - }): Promise { - const {channel, guild, hasPermission, member} = await this.deps.channelAuthService.getChannelAuthenticated({ + }): Promise { + const authChannel = await this.deps.channelAuthService.getChannelAuthenticated({ userId, channelId, }); + const {channel, guild, hasPermission, member} = authChannel; const hasNewAttachments = data.attachments?.some( (attachment) => @@ -131,7 +138,7 @@ export class MessageEditService { }); } if (message.authorId !== userId) { - return await this.withMessageLock(channelId, messageId, () => + const editedMessage = await this.withMessageLock(channelId, messageId, () => this.deps.processingService.handleNonAuthorEdit({ message, messageId, @@ -144,6 +151,7 @@ export class MessageEditService { dispatchService: this.deps.dispatchService, }), ); + return {message: editedMessage, authChannel}; } const isBugHunterBot = !!user?.isBot && (user.flags & UserFlags.BUG_HUNTER) !== 0n; const updateResult = await this.withMessageLock(channelId, messageId, () => @@ -184,7 +192,7 @@ export class MessageEditService { if (channel.indexedAt != null) { void this.deps.searchService.updateMessageIndex(updatedMessage); } - return updatedMessage; + return {message: updatedMessage, authChannel}; } private getEffectiveAllowedMentionsForEdit({ diff --git a/fluxer_api/src/api/channel/services/message/MessageRequestService.ts b/fluxer_api/src/api/channel/services/message/MessageRequestService.ts index 5407573df..92452caaa 100644 --- a/fluxer_api/src/api/channel/services/message/MessageRequestService.ts +++ b/fluxer_api/src/api/channel/services/message/MessageRequestService.ts @@ -106,7 +106,7 @@ export class MessageRequestService { ) { throw new UnclaimedAccountCannotSendMessagesError(); } - const message = await this.channelService.messages.send.sendMessage({ + const {message, authChannel} = await this.channelService.messages.send.sendMessage({ user: params.user, channelId: params.channelId, data: params.data, @@ -115,6 +115,7 @@ export class MessageRequestService { const access = await this.channelService.messages.retrieval.getResponseAccessContext({ userId: params.user.id, channelId: params.channelId, + authChannel, }); return this.responseDataService.buildMessage({ userId: params.user.id, @@ -132,7 +133,7 @@ export class MessageRequestService { data: MessageUpdateRequest; requestCache: RequestCache; }): Promise { - const message = await this.channelService.messages.edit.editMessage({ + const {message, authChannel} = await this.channelService.messages.edit.editMessage({ userId: params.userId, channelId: params.channelId, messageId: params.messageId, @@ -143,6 +144,7 @@ export class MessageRequestService { userId: params.userId, channelId: params.channelId, messageId: message.id, + authChannel, }); return this.responseDataService.buildMessage({ userId: params.userId, diff --git a/fluxer_api/src/api/channel/services/message/MessageRetrievalService.ts b/fluxer_api/src/api/channel/services/message/MessageRetrievalService.ts index 93696b00d..2ea59bc44 100644 --- a/fluxer_api/src/api/channel/services/message/MessageRetrievalService.ts +++ b/fluxer_api/src/api/channel/services/message/MessageRetrievalService.ts @@ -63,11 +63,14 @@ export class MessageRetrievalService { userId: UserID; channelId: ChannelID; messageId?: MessageID; + authChannel?: AuthenticatedChannel; }): Promise { - const authChannel = await this.channelAuthService.getChannelAuthenticated({ - userId: params.userId, - channelId: params.channelId, - }); + const authChannel = + params.authChannel ?? + (await this.channelAuthService.getChannelAuthenticated({ + userId: params.userId, + channelId: params.channelId, + })); if (params.messageId && !(await this.canAccessMessage(authChannel, params.messageId))) { throw new UnknownMessageError(); } diff --git a/fluxer_api/src/api/channel/services/message/MessageSendService.ts b/fluxer_api/src/api/channel/services/message/MessageSendService.ts index e005393b6..dd80dc07c 100644 --- a/fluxer_api/src/api/channel/services/message/MessageSendService.ts +++ b/fluxer_api/src/api/channel/services/message/MessageSendService.ts @@ -94,6 +94,11 @@ interface MessageSendServiceDeps { directMessageSpamMitigationService: DirectMessageSpamMitigationService; } +interface SendMessageResult { + message: Message; + authChannel: AuthenticatedChannel; +} + interface SendMentionData { flags: number; mentionUserIds: Array; @@ -783,7 +788,7 @@ export class MessageSendService { channelId: ChannelID; data: MessageRequest; requestCache: RequestCache; - }): Promise { + }): Promise { const authChannel = await this.deps.channelAuthService.getChannelAuthenticated({ userId: user.id, channelId, @@ -792,7 +797,8 @@ export class MessageSendService { throw InputValidationError.fromCode('content', ValidationErrorCodes.MUST_START_SESSION_BEFORE_SENDING); } if (isPersonalNotesChannel({userId: user.id, channelId})) { - return this.sendPersonalNoteMessage({user, channelId, data, requestCache}); + const message = await this.sendPersonalNoteMessage({authChannel, user, channelId, data, requestCache}); + return {message, authChannel}; } const {channel, guild, checkPermission, hasPermission, member} = authChannel; const {canEmbedLinks, canMentionEveryone, canAttachFiles} = await this.checkMessageSendPermissions({ @@ -820,7 +826,7 @@ export class MessageSendService { expectedChannelId: channelId, }); if (existingMessage) { - return existingMessage; + return {message: existingMessage, authChannel}; } const referenceContext = await this.resolveReferenceContext({ data, @@ -1031,7 +1037,7 @@ export class MessageSendService { if (searchIndexOptions && !suppressDmRecipientDelivery) { void this.deps.searchService.indexMessage(message, user.isBot, searchIndexOptions); } - return message; + return {message, authChannel}; } async sendWebhookMessage({ @@ -1270,17 +1276,19 @@ export class MessageSendService { } private async sendPersonalNoteMessage({ + authChannel, user, channelId, data, requestCache, }: { + authChannel: AuthenticatedChannel; user: User; channelId: ChannelID; data: MessageRequest; requestCache: RequestCache; }): Promise { - const {channel} = await this.deps.channelAuthService.getChannelAuthenticated({userId: user.id, channelId}); + const {channel} = authChannel; const isForwardMessage = this.ensureMessageRequestIsValid({user, data, guildFeatures: null}); this.deps.embedAttachmentResolver.validateAttachmentReferences({ embeds: data.embeds, diff --git a/fluxer_api/src/api/channel/tests/MessageSendPermissions.test.ts b/fluxer_api/src/api/channel/tests/MessageSendPermissions.test.ts index 1e67a1099..05998a4a5 100644 --- a/fluxer_api/src/api/channel/tests/MessageSendPermissions.test.ts +++ b/fluxer_api/src/api/channel/tests/MessageSendPermissions.test.ts @@ -3,9 +3,10 @@ import {Permissions} from '@fluxer/constants/src/ChannelConstants'; import {MAX_MESSAGE_LENGTH_PREMIUM} from '@fluxer/constants/src/LimitConstants'; import type {MessageResponse} from '@fluxer/schema/src/domains/message/MessageResponseSchemas'; -import {afterAll, beforeAll, beforeEach, describe, expect, it} from 'vitest'; +import {afterAll, beforeAll, beforeEach, describe, expect, it, vi} from 'vitest'; import {authorizeBot, createTestBotAccount} from '../../bot/tests/BotTestUtils'; import {ensureSessionStarted} from '../../message/tests/MessageTestUtils'; +import {getGatewayService} from '../../middleware/ServiceRegistry'; import {type ApiTestHarness, createApiTestHarness} from '../../test/ApiTestHarness'; import {HTTP_STATUS} from '../../test/TestConstants'; import {createBuilder} from '../../test/TestRequestBuilder'; @@ -46,6 +47,101 @@ describe('Message send permissions', () => { expect(sentMessage.author.id).toBe(member.userId); }); + it('rejects sending and editing when VIEW_CHANNEL is denied', async () => { + const {owner, members, systemChannel} = await setupTestGuildWithMembers(harness, 1); + const member = members[0]!; + await ensureSessionStarted(harness, member.token); + const sentMessage = await createBuilder(harness, member.token) + .post(`/channels/${systemChannel.id}/messages`) + .body({content: 'before the lockout'}) + .execute(); + await createPermissionOverwrite(harness, owner.token, systemChannel.id, member.userId, { + type: 1, + allow: '0', + deny: Permissions.VIEW_CHANNEL.toString(), + }); + + await createBuilder(harness, member.token) + .post(`/channels/${systemChannel.id}/messages`) + .body({content: 'after the lockout'}) + .expect(HTTP_STATUS.FORBIDDEN, 'MISSING_PERMISSIONS') + .execute(); + await createBuilder(harness, member.token) + .patch(`/channels/${systemChannel.id}/messages/${sentMessage.id}`) + .body({content: 'after the lockout'}) + .expect(HTTP_STATUS.FORBIDDEN, 'MISSING_PERMISSIONS') + .execute(); + }); + + it('hides the edited message when the editor cannot read history', async () => { + const {owner, members, systemChannel} = await setupTestGuildWithMembers(harness, 1); + const member = members[0]!; + await ensureSessionStarted(harness, member.token); + const sentMessage = await createBuilder(harness, member.token) + .post(`/channels/${systemChannel.id}/messages`) + .body({content: 'history is a luxury'}) + .execute(); + await createPermissionOverwrite(harness, owner.token, systemChannel.id, member.userId, { + type: 1, + allow: (Permissions.VIEW_CHANNEL | Permissions.SEND_MESSAGES).toString(), + deny: Permissions.READ_MESSAGE_HISTORY.toString(), + }); + + await createBuilder(harness, member.token) + .patch(`/channels/${systemChannel.id}/messages/${sentMessage.id}`) + .body({content: 'history is still a luxury'}) + .expect(HTTP_STATUS.NOT_FOUND, 'UNKNOWN_MESSAGE') + .execute(); + }); + + it('authenticates the channel once per send and once per edit', async () => { + const {owner, members, systemChannel} = await setupTestGuildWithMembers(harness, 1); + const member = members[0]!; + await createPermissionOverwrite(harness, owner.token, systemChannel.id, member.userId, { + type: 1, + allow: (Permissions.VIEW_CHANNEL | Permissions.SEND_MESSAGES | Permissions.READ_MESSAGE_HISTORY).toString(), + deny: '0', + }); + await ensureSessionStarted(harness, member.token); + const gatewayService = getGatewayService(); + const getGuildData = vi.spyOn(gatewayService, 'getGuildData'); + const getGuildMember = vi.spyOn(gatewayService, 'getGuildMember'); + const checkPermission = vi.spyOn(gatewayService, 'checkPermission'); + const countViewChannelChecks = () => + checkPermission.mock.calls.filter((call) => call[0].permission === Permissions.VIEW_CHANNEL).length; + + try { + const sentMessage = await createBuilder(harness, member.token) + .post(`/channels/${systemChannel.id}/messages`) + .body({content: 'authenticate me once'}) + .execute(); + const sendCounts = { + guildData: getGuildData.mock.calls.length, + guildMember: getGuildMember.mock.calls.length, + viewChannel: countViewChannelChecks(), + }; + getGuildData.mockClear(); + getGuildMember.mockClear(); + checkPermission.mockClear(); + await createBuilder(harness, member.token) + .patch(`/channels/${systemChannel.id}/messages/${sentMessage.id}`) + .body({content: 'authenticate me once again'}) + .execute(); + const editCounts = { + guildData: getGuildData.mock.calls.length, + guildMember: getGuildMember.mock.calls.length, + viewChannel: countViewChannelChecks(), + }; + + expect(sendCounts).toEqual({guildData: 1, guildMember: 1, viewChannel: 1}); + expect(editCounts).toEqual({guildData: 1, guildMember: 1, viewChannel: 1}); + } finally { + getGuildData.mockRestore(); + getGuildMember.mockRestore(); + checkPermission.mockRestore(); + } + }); + it('allows bot message content up to 4000 characters', async () => { const {owner, guild, systemChannel} = await setupTestGuildWithMembers(harness, 0); const botAccount = await createTestBotAccount(harness);