From ca56fd419b3131694d0ad5022e7fc4d09146ffb5 Mon Sep 17 00:00:00 2001 From: Ske Date: Wed, 10 Jul 2019 23:16:17 +0200 Subject: [PATCH] Fix various issues with proxying and webhook caching --- PluralKit.Bot/Bot.cs | 2 +- PluralKit.Bot/Services/EmbedService.cs | 2 +- PluralKit.Bot/Services/ProxyService.cs | 47 ++++++++++++++++++- PluralKit.Bot/Services/WebhookCacheService.cs | 17 +++++-- 4 files changed, 61 insertions(+), 7 deletions(-) diff --git a/PluralKit.Bot/Bot.cs b/PluralKit.Bot/Bot.cs index 3d565c81..71c04a39 100644 --- a/PluralKit.Bot/Bot.cs +++ b/PluralKit.Bot/Bot.cs @@ -51,7 +51,7 @@ namespace PluralKit.Bot .AddTransient(_ => _config.GetSection("PluralKit").Get() ?? new CoreConfig()) .AddTransient(_ => _config.GetSection("PluralKit").GetSection("Bot").Get() ?? new BotConfig()) - .AddScoped(svc => + .AddTransient(svc => { var conn = new NpgsqlConnection(svc.GetRequiredService().Database); diff --git a/PluralKit.Bot/Services/EmbedService.cs b/PluralKit.Bot/Services/EmbedService.cs index 2278684e..4e3ef3bd 100644 --- a/PluralKit.Bot/Services/EmbedService.cs +++ b/PluralKit.Bot/Services/EmbedService.cs @@ -46,7 +46,7 @@ namespace PluralKit.Bot { return new EmbedBuilder() .WithAuthor($"#{message.Channel.Name}: {member.Name}", member.AvatarUrl) .WithDescription(message.Content) - .WithFooter($"System ID: {system.Hid} | Member ID: {member.Hid} | Sender: {sender.Username}#{sender.Discriminator} ({sender.Id}) | Message ID: ${message.Id}") + .WithFooter($"System ID: {system.Hid} | Member ID: {member.Hid} | Sender: {sender.Username}#{sender.Discriminator} ({sender.Id}) | Message ID: {message.Id}") .WithTimestamp(message.Timestamp) .Build(); } diff --git a/PluralKit.Bot/Services/ProxyService.cs b/PluralKit.Bot/Services/ProxyService.cs index 67178d59..083461a4 100644 --- a/PluralKit.Bot/Services/ProxyService.cs +++ b/PluralKit.Bot/Services/ProxyService.cs @@ -77,12 +77,19 @@ namespace PluralKit.Bot } public async Task HandleMessageAsync(IMessage message) { - var results = await _connection.QueryAsync("select members.*, systems.* from members, systems, accounts where members.system = systems.id and accounts.system = systems.id and accounts.uid = @Uid", (member, system) => new ProxyDatabaseResult { Member = member, System = system }, new { Uid = message.Author.Id }); + var results = await _connection.QueryAsync( + "select members.*, systems.* from members, systems, accounts where members.system = systems.id and accounts.system = systems.id and accounts.uid = @Uid", + (member, system) => + new ProxyDatabaseResult { Member = member, System = system }, new { Uid = message.Author.Id }); // Find a member with proxy tags matching the message var match = GetProxyTagMatch(message.Content, results); if (match == null) return; + // We know message.Channel can only be ITextChannel as PK doesn't work in DMs/groups + // Afterwards we ensure the bot has the right permissions, otherwise bail early + if (!await EnsureBotPermissions(message.Channel as ITextChannel)) return; + // Fetch a webhook for this channel, and send the proxied message var webhook = await _webhookCache.GetWebhook(message.Channel as ITextChannel); var hookMessage = await ExecuteWebhook(webhook, match.InnerText, match.ProxyName, match.Member.AvatarUrl, message.Attachments.FirstOrDefault()); @@ -96,8 +103,42 @@ namespace PluralKit.Bot await message.DeleteAsync(); } + private async Task EnsureBotPermissions(ITextChannel channel) + { + var guildUser = await channel.Guild.GetCurrentUserAsync(); + var permissions = guildUser.GetPermissions(channel); + + if (!permissions.ManageWebhooks) + { + await channel.SendMessageAsync( + $"{Emojis.Error} PluralKit does not have the *Manage Webhooks* permission in this channel, and thus cannot proxy messages. Please contact a server administrator to remedy this."); + return false; + } + + if (!permissions.ManageMessages) + { + await channel.SendMessageAsync( + $"{Emojis.Error} PluralKit does not have the *Manage Messages* permission in this channel, and thus cannot delete the original trigger message. Please contact a server administrator to remedy this."); + return false; + } + + return true; + } + private async Task ExecuteWebhook(IWebhook webhook, string text, string username, string avatarUrl, IAttachment attachment) { - var client = new DiscordWebhookClient(webhook); + // TODO: DiscordWebhookClient's ctor does a call to GetWebhook that may be unnecessary, see if there's a way to do this The Hard Way :tm: + // TODO: this will probably crash if there are multiple consecutive failures, perhaps have a loop instead? + DiscordWebhookClient client; + try + { + client = new DiscordWebhookClient(webhook); + } + catch (InvalidOperationException) + { + // webhook was deleted or invalid + webhook = await _webhookCache.InvalidateAndRefreshWebhook(webhook); + client = new DiscordWebhookClient(webhook); + } ulong messageId; if (attachment != null) { @@ -108,6 +149,8 @@ namespace PluralKit.Bot } else { messageId = await client.SendMessageAsync(text, username: username, avatarUrl: avatarUrl); } + + // TODO: SendMessageAsync should return a full object(??), see if there's a way to avoid the extra server call here return await webhook.Channel.GetMessageAsync(messageId); } diff --git a/PluralKit.Bot/Services/WebhookCacheService.cs b/PluralKit.Bot/Services/WebhookCacheService.cs index e45597d0..8ea52dc7 100644 --- a/PluralKit.Bot/Services/WebhookCacheService.cs +++ b/PluralKit.Bot/Services/WebhookCacheService.cs @@ -32,13 +32,24 @@ namespace PluralKit.Bot // We cache the webhook through a Lazy>, this way we make sure to only create one webhook per channel // If the webhook is requested twice before it's actually been found, the Lazy wrapper will stop the // webhook from being created twice. - var lazyWebhookValue = + var lazyWebhookValue = _webhooks.GetOrAdd(channel.Id, new Lazy>(() => GetOrCreateWebhook(channel))); - return await lazyWebhookValue.Value; + + // It's possible to "move" a webhook to a different channel after creation + // Here, we ensure it's actually still pointing towards the proper channel, and if not, wipe and refetch one. + var webhook = await lazyWebhookValue.Value; + if (webhook.Channel.Id != channel.Id) return await InvalidateAndRefreshWebhook(webhook); + return webhook; + } + + public async Task InvalidateAndRefreshWebhook(IWebhook webhook) + { + _webhooks.TryRemove(webhook.Channel.Id, out _); + return await GetWebhook(webhook.Channel.Id); } private async Task GetOrCreateWebhook(ITextChannel channel) => - await FindExistingWebhook(channel) ?? await GetOrCreateWebhook(channel); + await FindExistingWebhook(channel) ?? await DoCreateWebhook(channel); private async Task FindExistingWebhook(ITextChannel channel) => (await channel.GetWebhooksAsync()).FirstOrDefault(IsWebhookMine);