fix(api): require manage messages to remove others' reactions (#2799)

This commit is contained in:
Hampus
2026-09-15 18:16:55 +02:00
committed by GitHub
parent 910db6734b
commit 570c8776c4
4 changed files with 144 additions and 27 deletions
@@ -223,7 +223,7 @@ export class MessageInteractionService {
emoji: string;
}): Promise<void> {
const authChannel = await this.authService.getChannelAuthenticated({userId, channelId});
await this.reactionService.removeAllReactionsForEmoji({authChannel, messageId, emoji, actorId: userId});
await this.reactionService.removeAllReactionsForEmoji({authChannel, messageId, emoji});
}
async removeAllReactions({
@@ -236,7 +236,7 @@ export class MessageInteractionService {
messageId: MessageID;
}): Promise<void> {
const authChannel = await this.authService.getChannelAuthenticated({userId, channelId});
await this.reactionService.removeAllReactions({authChannel, messageId, actorId: userId});
await this.reactionService.removeAllReactions({authChannel, messageId});
}
async getMessageReactions({
@@ -12,7 +12,6 @@ import type {LimitConfigService} from '@app/api/limits/LimitConfigService';
import {resolveLimitSafe} from '@app/api/limits/LimitConfigUtils';
import {createLimitMatchContext} from '@app/api/limits/LimitMatchContextBuilder';
import type {Channel} from '@app/api/models/Channel';
import type {Message} from '@app/api/models/Message';
import type {MessageReaction} from '@app/api/models/MessageReaction';
import type {User} from '@app/api/models/User';
import type {IUserRepository} from '@app/api/user/IUserRepository';
@@ -274,7 +273,7 @@ export class MessageReactionService extends MessageInteractionBase {
if (!message) return;
const isRemovingOwnReaction = targetId === actorId;
if (!isRemovingOwnReaction) {
await this.assertCanModerateMessageReactions({channel, message, actorId, hasPermission});
await this.assertCanModerateMessageReactions({channel, hasPermission});
}
const emojiId = parsedEmoji.id ? createEmojiID(BigInt(parsedEmoji.id)) : undefined;
await this.channelRepository.messageInteractions.removeReaction(
@@ -297,12 +296,10 @@ export class MessageReactionService extends MessageInteractionBase {
authChannel,
messageId,
emoji,
actorId,
}: {
authChannel: AuthenticatedChannel;
messageId: MessageID;
emoji: string;
actorId: UserID;
}): Promise<void> {
const channel = authChannel.channel;
const {guild, hasPermission} = authChannel;
@@ -314,7 +311,7 @@ export class MessageReactionService extends MessageInteractionBase {
const parsedEmoji = this.parseEmojiWithoutValidation(emoji);
const message = await this.channelRepository.messages.getMessage(channel.id, messageId);
if (!message) return;
await this.assertCanModerateMessageReactions({channel, message, actorId, hasPermission});
await this.assertCanModerateMessageReactions({channel, hasPermission});
const emojiId = parsedEmoji.id ? createEmojiID(BigInt(parsedEmoji.id)) : undefined;
await this.channelRepository.messageInteractions.removeAllReactionsForEmoji(
channel.id,
@@ -332,11 +329,9 @@ export class MessageReactionService extends MessageInteractionBase {
async removeAllReactions({
authChannel,
messageId,
actorId,
}: {
authChannel: AuthenticatedChannel;
messageId: MessageID;
actorId: UserID;
}): Promise<void> {
const channel = authChannel.channel;
const {guild, hasPermission} = authChannel;
@@ -347,7 +342,7 @@ export class MessageReactionService extends MessageInteractionBase {
}
const message = await this.channelRepository.messages.getMessage(channel.id, messageId);
if (!message) return;
await this.assertCanModerateMessageReactions({channel, message, actorId, hasPermission});
await this.assertCanModerateMessageReactions({channel, hasPermission});
await this.channelRepository.messageInteractions.removeAllReactions(channel.id, messageId);
await this.dispatchMessageReactionRemoveAll({channel, messageId});
}
@@ -365,18 +360,11 @@ export class MessageReactionService extends MessageInteractionBase {
private async assertCanModerateMessageReactions({
channel,
message,
actorId,
hasPermission,
}: {
channel: Channel;
message: Message;
actorId: UserID;
hasPermission: (permission: bigint) => Promise<boolean>;
}): Promise<void> {
if (message.authorId === actorId) {
return;
}
if (!channel.guildId) {
throw new MissingPermissionsError();
}
@@ -0,0 +1,132 @@
// SPDX-License-Identifier: AGPL-3.0-or-later
import {createTestAccount, type TestAccount} from '@app/api/auth/tests/AuthTestUtils';
import {
createDmChannel,
createFriendship,
createPermissionOverwrite,
sendChannelMessage,
setupTestGuildWithMembers,
} from '@app/api/channel/tests/ChannelTestUtils';
import {type ApiTestHarness, createApiTestHarness} from '@app/api/test/ApiTestHarness';
import {HTTP_STATUS} from '@app/api/test/TestConstants';
import {createBuilder} from '@app/api/test/TestRequestBuilder';
import {Permissions} from '@fluxer/constants/src/ChannelConstants';
import {afterAll, beforeAll, beforeEach, describe, expect, it} from 'vitest';
const EMOJI = encodeURIComponent('👍');
interface ReactionUsersPage {
items: Array<{id: string}>;
}
describe('Reaction removal permissions', () => {
let harness: ApiTestHarness;
beforeAll(async () => {
harness = await createApiTestHarness();
});
beforeEach(async () => {
await harness.reset();
});
afterAll(async () => {
await harness?.shutdown();
});
async function react(account: TestAccount, channelId: string, messageId: string): Promise<void> {
await createBuilder(harness, account.token)
.put(`/channels/${channelId}/messages/${messageId}/reactions/${EMOJI}/@me`)
.body(null)
.expect(HTTP_STATUS.NO_CONTENT)
.execute();
}
async function listReactorIds(account: TestAccount, channelId: string, messageId: string): Promise<Array<string>> {
const page = await createBuilder<ReactionUsersPage>(harness, account.token)
.get(`/channels/${channelId}/messages/${messageId}/reactions/${EMOJI}/users`)
.execute();
return page.items.map((user) => user.id);
}
async function expectRemovalsRejected(
account: TestAccount,
channelId: string,
messageId: string,
targetId: string,
): Promise<void> {
const base = `/channels/${channelId}/messages/${messageId}/reactions`;
for (const route of [`${base}/${EMOJI}/${targetId}`, `${base}/${EMOJI}`, base]) {
await createBuilder(harness, account.token)
.delete(route)
.expect(HTTP_STATUS.FORBIDDEN, 'MISSING_PERMISSIONS')
.execute();
}
}
it('requires MANAGE_MESSAGES for the author to remove reactions from their own guild message', async () => {
const {members, systemChannel} = await setupTestGuildWithMembers(harness, 2);
const [author, reactor] = members as [TestAccount, TestAccount];
const message = await sendChannelMessage(harness, author.token, systemChannel.id, 'react to me');
await react(reactor, systemChannel.id, message.id);
await expectRemovalsRejected(author, systemChannel.id, message.id, reactor.userId);
expect(await listReactorIds(author, systemChannel.id, message.id)).toEqual([reactor.userId]);
});
it('refuses the author removing reactions from their own direct message', async () => {
const author = await createTestAccount(harness);
const reactor = await createTestAccount(harness);
await createFriendship(harness, author, reactor);
const dm = await createDmChannel(harness, author.token, reactor.userId);
const message = await sendChannelMessage(harness, author.token, dm.id, 'react to me');
await react(reactor, dm.id, message.id);
await expectRemovalsRejected(author, dm.id, message.id, reactor.userId);
expect(await listReactorIds(author, dm.id, message.id)).toEqual([reactor.userId]);
});
it('lets a member with MANAGE_MESSAGES remove reactions from another member message', async () => {
const {owner, members, systemChannel} = await setupTestGuildWithMembers(harness, 3);
const [author, reactor, moderator] = members as [TestAccount, TestAccount, TestAccount];
await createPermissionOverwrite(harness, owner.token, systemChannel.id, moderator.userId, {
type: 1,
allow: Permissions.MANAGE_MESSAGES.toString(),
deny: '0',
});
const message = await sendChannelMessage(harness, author.token, systemChannel.id, 'react to me');
const base = `/channels/${systemChannel.id}/messages/${message.id}/reactions`;
await react(reactor, systemChannel.id, message.id);
await createBuilder(harness, moderator.token)
.delete(`${base}/${EMOJI}/${reactor.userId}`)
.expect(HTTP_STATUS.NO_CONTENT)
.execute();
expect(await listReactorIds(moderator, systemChannel.id, message.id)).toEqual([]);
await react(reactor, systemChannel.id, message.id);
await createBuilder(harness, moderator.token).delete(`${base}/${EMOJI}`).expect(HTTP_STATUS.NO_CONTENT).execute();
expect(await listReactorIds(moderator, systemChannel.id, message.id)).toEqual([]);
await react(reactor, systemChannel.id, message.id);
await createBuilder(harness, moderator.token).delete(base).expect(HTTP_STATUS.NO_CONTENT).execute();
expect(await listReactorIds(moderator, systemChannel.id, message.id)).toEqual([]);
});
it('lets a member without MANAGE_MESSAGES remove their own reaction by user ID', async () => {
const {members, systemChannel} = await setupTestGuildWithMembers(harness, 2);
const [author, reactor] = members as [TestAccount, TestAccount];
const message = await sendChannelMessage(harness, author.token, systemChannel.id, 'react to me');
await react(reactor, systemChannel.id, message.id);
await createBuilder(harness, reactor.token)
.delete(`/channels/${systemChannel.id}/messages/${message.id}/reactions/${EMOJI}/${reactor.userId}`)
.expect(HTTP_STATUS.NO_CONTENT)
.execute();
expect(await listReactorIds(author, systemChannel.id, message.id)).toEqual([]);
});
});
@@ -1920,8 +1920,7 @@ Removes one named user's reaction. Returns 204 with an empty body. Emits a [Mess
### Limitations
- The caller must be able to view the text-bearing channel and reach the message under the message history cutoff.
- The message author may remove any user's reaction from their own message in any channel.
- Any other caller must be in a guild channel and must hold [MANAGE_MESSAGES](/http-api/permissions/), so a non-author in a private channel is refused with 403 `MISSING_PERMISSIONS`.
- The caller must be in a guild channel and must hold [MANAGE_MESSAGES](/http-api/permissions/), so a caller in a private channel is refused with 403 `MISSING_PERMISSIONS`.
- `MANAGE_MESSAGES` is an [elevated permission](/http-api/permissions/#elevated-permissions), so a caller who holds it with no enrolled authenticator receives 400 [`TWO_FACTOR_REQUIRED`](/http-api/errors/) in a guild whose [MFA level](/http-api/guilds/#mfa-levels) is elevated, unless they own the guild.
### Path parameters
@@ -1949,10 +1948,10 @@ Removes one named user's reaction. Returns 204 with an empty body. Emits a [Mess
| --- | --- | --- |
| 204 | empty | Target reaction was absent or removed |
| 400 | [error response](/http-api/#error-response) | Path, emoji, or session input is invalid, or the channel has no messages and the request returns `CANNOT_SEND_MESSAGES_IN_NON_TEXT_CHANNEL` |
| 403 | [error response](/http-api/#error-response) | Caller is not the message author and lacks `VIEW_CHANNEL` or `MANAGE_MESSAGES`, or is not the author in a private channel, each returning `MISSING_PERMISSIONS`, or reactions are temporarily disabled for the guild and the request returns `FEATURE_TEMPORARILY_DISABLED` |
| 403 | [error response](/http-api/#error-response) | Caller lacks `VIEW_CHANNEL` or `MANAGE_MESSAGES`, or the channel is private, each returning `MISSING_PERMISSIONS`, or reactions are temporarily disabled for the guild and the request returns `FEATURE_TEMPORARILY_DISABLED` |
| 404 | [error response](/http-api/#error-response) | Channel does not exist, or the message is outside the caller's message history cutoff |
A message that does not exist returns 204 before the authorship and permission checks. A request naming a target user with no such reaction also returns 204.
A message that does not exist returns 204 before the `MANAGE_MESSAGES` check. A request naming a target user with no such reaction also returns 204.
### Side effects
@@ -1971,8 +1970,7 @@ Removes every reaction for one emoji. Returns 204 with an empty body. Emits a [M
### Limitations
- The caller must be able to view the text-bearing channel and reach the message under the message history cutoff.
- The message author may clear the emoji on their own message in any channel.
- Any other caller must be in a guild channel and must hold [MANAGE_MESSAGES](/http-api/permissions/), an [elevated permission](/http-api/permissions/#elevated-permissions).
- The caller must be in a guild channel and must hold [MANAGE_MESSAGES](/http-api/permissions/), an [elevated permission](/http-api/permissions/#elevated-permissions).
- A caller who holds `MANAGE_MESSAGES` with no enrolled authenticator receives 400 [`TWO_FACTOR_REQUIRED`](/http-api/errors/) in a guild whose [MFA level](/http-api/guilds/#mfa-levels) is elevated, unless they own the guild.
### Path parameters
@@ -1989,7 +1987,7 @@ Removes every reaction for one emoji. Returns 204 with an empty body. Emits a [M
| --- | --- | --- |
| 204 | empty | Emoji reactions were absent or removed |
| 400 | [error response](/http-api/#error-response) | Path or emoji is invalid, or the channel has no messages and the request returns `CANNOT_SEND_MESSAGES_IN_NON_TEXT_CHANNEL` |
| 403 | [error response](/http-api/#error-response) | Caller is not the message author and lacks `VIEW_CHANNEL` or `MANAGE_MESSAGES`, or is not the author in a private channel, each returning `MISSING_PERMISSIONS`, or reactions are temporarily disabled for the guild and the request returns `FEATURE_TEMPORARILY_DISABLED` |
| 403 | [error response](/http-api/#error-response) | Caller lacks `VIEW_CHANNEL` or `MANAGE_MESSAGES`, or the channel is private, each returning `MISSING_PERMISSIONS`, or reactions are temporarily disabled for the guild and the request returns `FEATURE_TEMPORARILY_DISABLED` |
| 404 | [error response](/http-api/#error-response) | Channel does not exist, or the message is outside the caller's message history cutoff |
### Side effects
@@ -2009,8 +2007,7 @@ Removes every reaction from a message. Returns 204 with an empty body. Emits a [
### Limitations
- The caller must be able to view the text-bearing channel and reach the message under the message history cutoff.
- The message author may clear their own message in any channel.
- Any other caller must be in a guild channel and must hold [MANAGE_MESSAGES](/http-api/permissions/), an [elevated permission](/http-api/permissions/#elevated-permissions).
- The caller must be in a guild channel and must hold [MANAGE_MESSAGES](/http-api/permissions/), an [elevated permission](/http-api/permissions/#elevated-permissions).
- A caller who holds `MANAGE_MESSAGES` with no enrolled authenticator receives 400 [`TWO_FACTOR_REQUIRED`](/http-api/errors/) in a guild whose [MFA level](/http-api/guilds/#mfa-levels) is elevated, unless they own the guild.
### Path parameters
@@ -2026,7 +2023,7 @@ Removes every reaction from a message. Returns 204 with an empty body. Emits a [
| --- | --- | --- |
| 204 | empty | Reactions were absent or removed |
| 400 | [error response](/http-api/#error-response) | Path parameters are invalid, or the channel has no messages and the request returns `CANNOT_SEND_MESSAGES_IN_NON_TEXT_CHANNEL` |
| 403 | [error response](/http-api/#error-response) | Caller is not the message author and lacks `VIEW_CHANNEL` or `MANAGE_MESSAGES`, or is not the author in a private channel, each returning `MISSING_PERMISSIONS`, or reactions are temporarily disabled for the guild and the request returns `FEATURE_TEMPORARILY_DISABLED` |
| 403 | [error response](/http-api/#error-response) | Caller lacks `VIEW_CHANNEL` or `MANAGE_MESSAGES`, or the channel is private, each returning `MISSING_PERMISSIONS`, or reactions are temporarily disabled for the guild and the request returns `FEATURE_TEMPORARILY_DISABLED` |
| 404 | [error response](/http-api/#error-response) | Channel does not exist, or the message is outside the caller's message history cutoff |
### Side effects