perf(message): reuse channel auth across send and edit (#2135)

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<Message> {
+	}): Promise<EditMessageResult> {
-		const {channel, guild, hasPermission, member} = await this.deps.channelAuthService.getChannelAuthenticated({
+		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({

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<MessageResponse> {
-		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,

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<MessageResponseAccessContext> {
-		const authChannel = await this.channelAuthService.getChannelAuthenticated({
+		const authChannel =
-			userId: params.userId,
+			params.authChannel ??
-			channelId: params.channelId,
+			(await this.channelAuthService.getChannelAuthenticated({
-		});
+				userId: params.userId,
+				channelId: params.channelId,
+			}));
 		if (params.messageId && !(await this.canAccessMessage(authChannel, params.messageId))) {
 			throw new UnknownMessageError();
 		}

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<UserID>;
@@ -783,7 +788,7 @@ export class MessageSendService {
 		channelId: ChannelID;
 		data: MessageRequest;
 		requestCache: RequestCache;
-	}): Promise<Message> {
+	}): Promise<SendMessageResult> {
 		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<Message> {
-		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,

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<MessageResponse>(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<MessageResponse>(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<MessageResponse>(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<MessageResponse>(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);