fix(channels): restrict Discord picker to postable channels the bot can use
Only text (0) and announcement (5) channels accept a direct message — forum (15) and media (16) require thread creation, so they're dropped from the picker (they returned 'non-text channel'). Channels are also filtered by the bot's effective VIEW_CHANNEL + SEND_MESSAGES, computed from role base perms and channel overwrites (Discord's documented algorithm), with a safe fallback to type-only filtering when the bot's roles can't be resolved.
This commit is contained in:
parent
df81532c67
commit
bf0494fbe6
4 changed files with 190 additions and 8 deletions
|
|
@ -19,19 +19,28 @@ class DiscordClient
|
|||
use HasSocialHttpClient;
|
||||
|
||||
/**
|
||||
* Text-postable channel types: 0 = text, 5 = announcement, 15 = forum.
|
||||
* Channel types that accept a direct message via POST /channels/{id}/messages:
|
||||
* 0 = text, 5 = announcement. Forum (15) is excluded — it only accepts a thread
|
||||
* (forum post), so a plain message returns "Cannot send messages in a non-text channel".
|
||||
*/
|
||||
private const POSTABLE_CHANNEL_TYPES = [0, 5, 15];
|
||||
private const POSTABLE_CHANNEL_TYPES = [0, 5];
|
||||
|
||||
private const CHANNELS_TTL = 300;
|
||||
|
||||
private const PERMISSION_ADMINISTRATOR = 0x8;
|
||||
|
||||
private const PERMISSION_VIEW_CHANNEL = 0x400;
|
||||
|
||||
private const PERMISSION_SEND_MESSAGES = 0x800;
|
||||
|
||||
public function baseUrl(): string
|
||||
{
|
||||
return (string) config('trypost.platforms.discord.api');
|
||||
}
|
||||
|
||||
/**
|
||||
* The text channels of a guild the bot can post into.
|
||||
* The channels of a guild the bot can actually post into: postable type
|
||||
* (text/announcement) AND the bot has VIEW_CHANNEL + SEND_MESSAGES there.
|
||||
*
|
||||
* @return list<array{id: string, name: string}>
|
||||
*
|
||||
|
|
@ -58,8 +67,12 @@ public function channels(string $guildId): array
|
|||
return [];
|
||||
}
|
||||
|
||||
$rolePermissions = $this->guildRolePermissions($guildId);
|
||||
$botRoleIds = $this->botRoleIds($guildId);
|
||||
|
||||
return collect($channels)
|
||||
->filter(fn ($channel) => in_array((int) data_get($channel, 'type'), self::POSTABLE_CHANNEL_TYPES, true))
|
||||
->filter(fn ($channel) => $this->botCanPostInChannel($channel, $guildId, $rolePermissions, $botRoleIds))
|
||||
->map(fn ($channel) => [
|
||||
'id' => (string) data_get($channel, 'id'),
|
||||
'name' => (string) data_get($channel, 'name'),
|
||||
|
|
@ -69,6 +82,115 @@ public function channels(string $guildId): array
|
|||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the bot has VIEW_CHANNEL + SEND_MESSAGES in a channel, computed from
|
||||
* its roles' base permissions and the channel's permission overwrites (Discord's
|
||||
* standard algorithm). Falls back to `true` when the bot's roles can't be
|
||||
* resolved, so a transient API hiccup never hides every channel from the picker.
|
||||
*
|
||||
* @param array<string, mixed> $channel
|
||||
* @param array<string, int> $rolePermissions role id => base permission bitfield
|
||||
* @param list<string>|null $botRoleIds role ids assigned to the bot, or null when unknown
|
||||
*/
|
||||
private function botCanPostInChannel(array $channel, string $guildId, array $rolePermissions, ?array $botRoleIds): bool
|
||||
{
|
||||
if ($botRoleIds === null || $rolePermissions === []) {
|
||||
return true;
|
||||
}
|
||||
|
||||
$base = $rolePermissions[$guildId] ?? 0; // @everyone role id == guild id
|
||||
foreach ($botRoleIds as $roleId) {
|
||||
$base |= ($rolePermissions[$roleId] ?? 0);
|
||||
}
|
||||
|
||||
if (($base & self::PERMISSION_ADMINISTRATOR) !== 0) {
|
||||
return true;
|
||||
}
|
||||
|
||||
$permissions = $this->applyChannelOverwrites($base, $channel, $guildId, $botRoleIds);
|
||||
|
||||
return ($permissions & self::PERMISSION_VIEW_CHANNEL) !== 0
|
||||
&& ($permissions & self::PERMISSION_SEND_MESSAGES) !== 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Applies a channel's permission_overwrites to a base bitfield in Discord's
|
||||
* documented order: @everyone overwrite, then the union of role overwrites,
|
||||
* then the bot's member overwrite.
|
||||
*
|
||||
* @param array<string, mixed> $channel
|
||||
* @param list<string> $botRoleIds
|
||||
*/
|
||||
private function applyChannelOverwrites(int $permissions, array $channel, string $guildId, array $botRoleIds): int
|
||||
{
|
||||
$overwrites = collect((array) data_get($channel, 'permission_overwrites', []));
|
||||
|
||||
$everyone = $overwrites->first(fn ($overwrite) => (string) data_get($overwrite, 'id') === $guildId);
|
||||
if ($everyone !== null) {
|
||||
$permissions = ($permissions & ~(int) data_get($everyone, 'deny', 0)) | (int) data_get($everyone, 'allow', 0);
|
||||
}
|
||||
|
||||
$roleAllow = 0;
|
||||
$roleDeny = 0;
|
||||
foreach ($overwrites as $overwrite) {
|
||||
if ((int) data_get($overwrite, 'type') === 0 && in_array((string) data_get($overwrite, 'id'), $botRoleIds, true)) {
|
||||
$roleAllow |= (int) data_get($overwrite, 'allow', 0);
|
||||
$roleDeny |= (int) data_get($overwrite, 'deny', 0);
|
||||
}
|
||||
}
|
||||
$permissions = ($permissions & ~$roleDeny) | $roleAllow;
|
||||
|
||||
$botId = (string) config('services.discord.client_id');
|
||||
$member = $overwrites->first(fn ($overwrite) => (int) data_get($overwrite, 'type') === 1 && (string) data_get($overwrite, 'id') === $botId);
|
||||
if ($member !== null) {
|
||||
$permissions = ($permissions & ~(int) data_get($member, 'deny', 0)) | (int) data_get($member, 'allow', 0);
|
||||
}
|
||||
|
||||
return $permissions;
|
||||
}
|
||||
|
||||
/**
|
||||
* Base permission bitfield per guild role (role id => permissions). Returns []
|
||||
* when the roles can't be fetched, which disables permission filtering.
|
||||
*
|
||||
* @return array<string, int>
|
||||
*/
|
||||
private function guildRolePermissions(string $guildId): array
|
||||
{
|
||||
$permissions = [];
|
||||
|
||||
foreach ($this->getList("{$this->baseUrl()}/guilds/{$guildId}/roles") as $role) {
|
||||
$permissions[(string) data_get($role, 'id')] = (int) data_get($role, 'permissions', 0);
|
||||
}
|
||||
|
||||
return $permissions;
|
||||
}
|
||||
|
||||
/**
|
||||
* The role ids assigned to the bot in a guild, or null when they can't be
|
||||
* resolved (no client id configured, or the member lookup failed).
|
||||
*
|
||||
* @return list<string>|null
|
||||
*/
|
||||
private function botRoleIds(string $guildId): ?array
|
||||
{
|
||||
$botId = (string) config('services.discord.client_id');
|
||||
|
||||
if ($botId === '') {
|
||||
return null;
|
||||
}
|
||||
|
||||
$response = $this->bot()->get("{$this->baseUrl()}/guilds/{$guildId}/members/{$botId}");
|
||||
|
||||
if ($response->failed()) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$roles = $response->json('roles');
|
||||
|
||||
return is_array($roles) ? array_map('strval', $roles) : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Mentionable targets matching a query: @everyone/@here, roles, then members.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -91,7 +91,7 @@ private function guardChannelBelongsToGuild(string $guildId, string $channelId):
|
|||
|
||||
if (! $allowed) {
|
||||
throw new DiscordPublishException(
|
||||
userMessage: 'The selected channel is not part of this Discord server.',
|
||||
userMessage: 'The bot can\'t post in the selected channel. Make sure it still exists, belongs to this server, and the bot has permission to send messages there.',
|
||||
category: ErrorCategory::Permission,
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,7 +11,10 @@
|
|||
|
||||
beforeEach(function () {
|
||||
Cache::flush(); // channel list is cached per guild — isolate between tests
|
||||
config(['trypost.platforms.discord.bot_token' => 'BOTTOKEN']);
|
||||
config([
|
||||
'trypost.platforms.discord.bot_token' => 'BOTTOKEN',
|
||||
'services.discord.client_id' => '999000111', // bot user id, used by the channel permission check
|
||||
]);
|
||||
|
||||
$this->user = User::factory()->create();
|
||||
$this->workspace = Workspace::factory()->create(['account_id' => $this->user->account_id, 'user_id' => $this->user->id]);
|
||||
|
|
@ -25,14 +28,20 @@
|
|||
]);
|
||||
});
|
||||
|
||||
test('lists only postable text channels of the guild', function () {
|
||||
test('lists only postable channels (text + announcement) the bot can send in', function () {
|
||||
Http::fake([
|
||||
config('trypost.platforms.discord.api').'/guilds/111222333/channels' => Http::response([
|
||||
['id' => '1', 'name' => 'general', 'type' => 0],
|
||||
['id' => '2', 'name' => 'voice', 'type' => 2], // voice — excluded
|
||||
['id' => '2', 'name' => 'voice', 'type' => 2], // voice — excluded
|
||||
['id' => '3', 'name' => 'news', 'type' => 5],
|
||||
['id' => '4', 'name' => 'category', 'type' => 4], // category — excluded
|
||||
['id' => '5', 'name' => 'forum', 'type' => 15], // forum — excluded (only accepts threads)
|
||||
], 200),
|
||||
// @everyone grants VIEW_CHANNEL + SEND_MESSAGES (1024 + 2048) everywhere.
|
||||
config('trypost.platforms.discord.api').'/guilds/111222333/roles' => Http::response([
|
||||
['id' => '111222333', 'name' => '@everyone', 'permissions' => '3072'],
|
||||
], 200),
|
||||
config('trypost.platforms.discord.api').'/guilds/111222333/members/999000111' => Http::response(['roles' => []], 200),
|
||||
]);
|
||||
|
||||
$response = $this->actingAs($this->user)
|
||||
|
|
@ -42,6 +51,47 @@
|
|||
expect(collect($response->json('channels'))->pluck('name')->all())->toBe(['general', 'news']);
|
||||
});
|
||||
|
||||
test('excludes channels where the bot lacks send permission', function () {
|
||||
Http::fake([
|
||||
config('trypost.platforms.discord.api').'/guilds/111222333/channels' => Http::response([
|
||||
['id' => '1', 'name' => 'open', 'type' => 0],
|
||||
['id' => '2', 'name' => 'locked', 'type' => 0, 'permission_overwrites' => [
|
||||
// @everyone overwrite denies SEND_MESSAGES (2048) on this channel.
|
||||
['id' => '111222333', 'type' => 0, 'allow' => '0', 'deny' => '2048'],
|
||||
]],
|
||||
], 200),
|
||||
config('trypost.platforms.discord.api').'/guilds/111222333/roles' => Http::response([
|
||||
['id' => '111222333', 'name' => '@everyone', 'permissions' => '3072'],
|
||||
], 200),
|
||||
config('trypost.platforms.discord.api').'/guilds/111222333/members/999000111' => Http::response(['roles' => []], 200),
|
||||
]);
|
||||
|
||||
$response = $this->actingAs($this->user)
|
||||
->getJson(route('app.discord.channels', $this->account->id));
|
||||
|
||||
$response->assertOk();
|
||||
expect(collect($response->json('channels'))->pluck('name')->all())->toBe(['open']);
|
||||
});
|
||||
|
||||
test('does not hide channels when the bot role lookup is unavailable', function () {
|
||||
// A failed members lookup must not blank the picker — fall back to type filtering.
|
||||
Http::fake([
|
||||
config('trypost.platforms.discord.api').'/guilds/111222333/channels' => Http::response([
|
||||
['id' => '1', 'name' => 'general', 'type' => 0],
|
||||
], 200),
|
||||
config('trypost.platforms.discord.api').'/guilds/111222333/roles' => Http::response([
|
||||
['id' => '111222333', 'name' => '@everyone', 'permissions' => '3072'],
|
||||
], 200),
|
||||
config('trypost.platforms.discord.api').'/guilds/111222333/members/999000111' => Http::response(['message' => 'Missing Access', 'code' => 50001], 403),
|
||||
]);
|
||||
|
||||
$response = $this->actingAs($this->user)
|
||||
->getJson(route('app.discord.channels', $this->account->id));
|
||||
|
||||
$response->assertOk();
|
||||
expect(collect($response->json('channels'))->pluck('name')->all())->toBe(['general']);
|
||||
});
|
||||
|
||||
test('returns mention targets: specials, roles and members', function () {
|
||||
Http::fake([
|
||||
config('trypost.platforms.discord.api').'/guilds/111222333/roles' => Http::response([
|
||||
|
|
|
|||
|
|
@ -18,7 +18,10 @@
|
|||
|
||||
beforeEach(function () {
|
||||
Cache::flush(); // channel list is cached per guild — isolate between tests
|
||||
config(['trypost.platforms.discord.bot_token' => 'BOTTOKEN']);
|
||||
config([
|
||||
'trypost.platforms.discord.bot_token' => 'BOTTOKEN',
|
||||
'services.discord.client_id' => '999000111', // bot user id, used by the channel permission check
|
||||
]);
|
||||
|
||||
$this->user = User::factory()->create();
|
||||
$this->workspace = Workspace::factory()->create(['user_id' => $this->user->id]);
|
||||
|
|
@ -51,6 +54,11 @@ function fakeDiscord(array $messageResponse = ['id' => '777'], int $status = 200
|
|||
config('trypost.platforms.discord.api').'/guilds/*/channels' => Http::response([
|
||||
['id' => '444555666', 'name' => 'general', 'type' => 0],
|
||||
], 200),
|
||||
// @everyone (role id == guild id) grants VIEW_CHANNEL + SEND_MESSAGES (1024 + 2048).
|
||||
config('trypost.platforms.discord.api').'/guilds/*/roles' => Http::response([
|
||||
['id' => '111222333', 'name' => '@everyone', 'permissions' => '3072'],
|
||||
], 200),
|
||||
config('trypost.platforms.discord.api').'/guilds/*/members/*' => Http::response(['roles' => []], 200),
|
||||
config('trypost.platforms.discord.api').'/channels/*/messages' => Http::response($messageResponse, $status),
|
||||
]);
|
||||
}
|
||||
|
|
@ -140,6 +148,8 @@ function fakeDiscord(array $messageResponse = ['id' => '777'], int $status = 200
|
|||
|
||||
Http::fake([
|
||||
config('trypost.platforms.discord.api').'/guilds/*/channels' => Http::response([['id' => '444555666', 'name' => 'general', 'type' => 0]], 200),
|
||||
config('trypost.platforms.discord.api').'/guilds/*/roles' => Http::response([['id' => '111222333', 'name' => '@everyone', 'permissions' => '3072']], 200),
|
||||
config('trypost.platforms.discord.api').'/guilds/*/members/*' => Http::response(['roles' => []], 200),
|
||||
'example.com/*' => Http::response(str_repeat('x', 1024), 200),
|
||||
config('trypost.platforms.discord.api').'/channels/*/messages' => Http::response(['id' => '901'], 200),
|
||||
]);
|
||||
|
|
|
|||
Loading…
Reference in a new issue