From f37cc49bb43a500ee18e42aed4ba2bb10483c90d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Spie=C3=9F?= Date: Sun, 12 May 2024 12:47:26 +0200 Subject: [PATCH 1/2] Make channel access checks consistent --- .../channel/concrete/PrivateChannelImpl.java | 3 -- .../channel/concrete/VoiceChannelImpl.java | 2 +- .../mixin/middleman/AudioChannelMixin.java | 12 ++++++++ .../mixin/middleman/GuildChannelMixin.java | 9 ++++++ .../middleman/GuildMessageChannelMixin.java | 12 ++++---- .../mixin/middleman/MessageChannelMixin.java | 29 ------------------- 6 files changed, 29 insertions(+), 38 deletions(-) diff --git a/src/main/java/net/dv8tion/jda/internal/entities/channel/concrete/PrivateChannelImpl.java b/src/main/java/net/dv8tion/jda/internal/entities/channel/concrete/PrivateChannelImpl.java index 57fc210aa9..cc8d32508a 100644 --- a/src/main/java/net/dv8tion/jda/internal/entities/channel/concrete/PrivateChannelImpl.java +++ b/src/main/java/net/dv8tion/jda/internal/entities/channel/concrete/PrivateChannelImpl.java @@ -114,9 +114,6 @@ public boolean canTalk() return user == null || !user.isBot(); } - @Override - public void checkCanAccessChannel() {} - @Override public void checkCanSendMessage() { checkBot(); diff --git a/src/main/java/net/dv8tion/jda/internal/entities/channel/concrete/VoiceChannelImpl.java b/src/main/java/net/dv8tion/jda/internal/entities/channel/concrete/VoiceChannelImpl.java index 158da44ae9..5f28819a10 100644 --- a/src/main/java/net/dv8tion/jda/internal/entities/channel/concrete/VoiceChannelImpl.java +++ b/src/main/java/net/dv8tion/jda/internal/entities/channel/concrete/VoiceChannelImpl.java @@ -179,7 +179,7 @@ public String getStatus() public AuditableRestAction modifyStatus(@Nonnull String status) { Checks.notLonger(status, MAX_STATUS_LENGTH, "Voice Status"); - checkCanAccessChannel(); + checkCanAccess(); if (this.equals(getGuild().getSelfMember().getVoiceState().getChannel())) checkPermission(Permission.VOICE_SET_STATUS); else diff --git a/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/AudioChannelMixin.java b/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/AudioChannelMixin.java index 90ccf18753..60b1e02b2e 100644 --- a/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/AudioChannelMixin.java +++ b/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/AudioChannelMixin.java @@ -17,8 +17,10 @@ package net.dv8tion.jda.internal.entities.channel.mixin.middleman; import gnu.trove.map.TLongObjectMap; +import net.dv8tion.jda.api.Permission; import net.dv8tion.jda.api.entities.Member; import net.dv8tion.jda.api.entities.channel.unions.AudioChannelUnion; +import net.dv8tion.jda.api.exceptions.MissingAccessException; public interface AudioChannelMixin> extends AudioChannelUnion, StandardGuildChannelMixin @@ -31,4 +33,14 @@ public interface AudioChannelMixin> T setUserLimit(int userlimit); T setRegion(String region); + + // AudioChannels also require connect permission to grant access + @Override + default void checkCanAccess() + { + if (!hasPermission(Permission.VIEW_CHANNEL)) + throw new MissingAccessException(this, Permission.VIEW_CHANNEL); + if (!hasPermission(Permission.VOICE_CONNECT)) + throw new MissingAccessException(this, Permission.VOICE_CONNECT); + } } diff --git a/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/GuildChannelMixin.java b/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/GuildChannelMixin.java index c157e3a207..bd2825a1bb 100644 --- a/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/GuildChannelMixin.java +++ b/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/GuildChannelMixin.java @@ -21,6 +21,7 @@ import net.dv8tion.jda.api.entities.channel.middleman.GuildChannel; import net.dv8tion.jda.api.entities.channel.unions.GuildChannelUnion; import net.dv8tion.jda.api.exceptions.InsufficientPermissionException; +import net.dv8tion.jda.api.exceptions.MissingAccessException; import net.dv8tion.jda.api.requests.Route; import net.dv8tion.jda.api.requests.restaction.AuditableRestAction; import net.dv8tion.jda.internal.entities.channel.mixin.ChannelMixin; @@ -40,6 +41,7 @@ public interface GuildChannelMixin> extends @CheckReturnValue default AuditableRestAction delete() { + checkCanAccess(); checkCanManage(); Route.CompiledRoute route = Route.Channels.DELETE_CHANNEL.compile(getId()); @@ -70,4 +72,11 @@ default void checkCanManage() { checkPermission(Permission.MANAGE_CHANNEL); } + + // Overridden by AudioChannelMixin + default void checkCanAccess() + { + if (!hasPermission(Permission.VIEW_CHANNEL)) + throw new MissingAccessException(this, Permission.VIEW_CHANNEL); + } } diff --git a/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/GuildMessageChannelMixin.java b/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/GuildMessageChannelMixin.java index d9578d8eca..0314fb724e 100644 --- a/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/GuildMessageChannelMixin.java +++ b/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/GuildMessageChannelMixin.java @@ -108,7 +108,6 @@ default RestAction clearReactionsById(@Nonnull String messageId, @Nonnull @Override default MessageCreateAction sendStickers(@Nonnull Collection stickers) { - checkCanAccessChannel(); checkCanSendMessage(); Checks.notEmpty(stickers, "Stickers"); Checks.noneNull(stickers, "Stickers"); @@ -116,13 +115,10 @@ default MessageCreateAction sendStickers(@Nonnull Collection embeds) { - checkCanAccessChannel(); checkCanSendMessage(); checkCanSendMessageEmbeds(); return MessageChannelUnion.super.sendMessageEmbeds(embeds); @@ -161,7 +158,6 @@ default MessageCreateAction sendMessageEmbeds(@Nonnull Collection components) { - checkCanAccessChannel(); checkCanSendMessage(); return MessageChannelUnion.super.sendMessageComponents(components); } @@ -179,7 +174,6 @@ default MessageCreateAction sendMessageComponents(@Nonnull Collection files) { - checkCanAccessChannel(); checkCanSendMessage(); checkCanSendFiles(); return MessageChannelUnion.super.sendFiles(files); @@ -207,7 +199,6 @@ default MessageCreateAction sendFiles(@Nonnull Collection @CheckReturnValue default RestAction retrieveMessageById(@Nonnull String messageId) { - checkCanAccessChannel(); checkCanViewHistory(); return MessageChannelUnion.super.retrieveMessageById(messageId); } @@ -216,7 +207,6 @@ default RestAction retrieveMessageById(@Nonnull String messageId) @CheckReturnValue default AuditableRestAction deleteMessageById(@Nonnull String messageId) { - checkCanAccessChannel(); //We don't know if this is a Message sent by us or another user, so we can't run checks for Permission.MESSAGE_MANAGE return MessageChannelUnion.super.deleteMessageById(messageId); } @@ -225,7 +215,6 @@ default AuditableRestAction deleteMessageById(@Nonnull String messageId) @Override default MessageHistory getHistory() { - checkCanAccessChannel(); checkCanViewHistory(); return MessageChannelUnion.super.getHistory(); } @@ -234,7 +223,6 @@ default MessageHistory getHistory() @CheckReturnValue default MessagePaginationAction getIterableHistory() { - checkCanAccessChannel(); checkCanViewHistory(); return MessageChannelUnion.super.getIterableHistory(); } @@ -243,7 +231,6 @@ default MessagePaginationAction getIterableHistory() @CheckReturnValue default MessageHistory.MessageRetrieveAction getHistoryAround(@Nonnull String messageId, int limit) { - checkCanAccessChannel(); checkCanViewHistory(); return MessageChannelUnion.super.getHistoryAround(messageId, limit); } @@ -252,7 +239,6 @@ default MessageHistory.MessageRetrieveAction getHistoryAround(@Nonnull String me @CheckReturnValue default MessageHistory.MessageRetrieveAction getHistoryAfter(@Nonnull String messageId, int limit) { - checkCanAccessChannel(); checkCanViewHistory(); return MessageChannelUnion.super.getHistoryAfter(messageId, limit); } @@ -261,7 +247,6 @@ default MessageHistory.MessageRetrieveAction getHistoryAfter(@Nonnull String mes @CheckReturnValue default MessageHistory.MessageRetrieveAction getHistoryBefore(@Nonnull String messageId, int limit) { - checkCanAccessChannel(); checkCanViewHistory(); return MessageChannelUnion.super.getHistoryBefore(messageId, limit); } @@ -270,7 +255,6 @@ default MessageHistory.MessageRetrieveAction getHistoryBefore(@Nonnull String me @CheckReturnValue default MessageHistory.MessageRetrieveAction getHistoryFromBeginning(int limit) { - checkCanAccessChannel(); checkCanViewHistory(); return MessageHistory.getHistoryFromBeginning(this).limit(limit); } @@ -279,7 +263,6 @@ default MessageHistory.MessageRetrieveAction getHistoryFromBeginning(int limit) @CheckReturnValue default RestAction sendTyping() { - checkCanAccessChannel(); return MessageChannelUnion.super.sendTyping(); } @@ -287,7 +270,6 @@ default RestAction sendTyping() @CheckReturnValue default RestAction addReactionById(@Nonnull String messageId, @Nonnull Emoji emoji) { - checkCanAccessChannel(); checkCanAddReactions(); return MessageChannelUnion.super.addReactionById(messageId, emoji); } @@ -296,7 +278,6 @@ default RestAction addReactionById(@Nonnull String messageId, @Nonnull Emo @CheckReturnValue default RestAction removeReactionById(@Nonnull String messageId, @Nonnull Emoji emoji) { - checkCanAccessChannel(); checkCanRemoveReactions(); return MessageChannelUnion.super.removeReactionById(messageId, emoji); } @@ -305,7 +286,6 @@ default RestAction removeReactionById(@Nonnull String messageId, @Nonnull @CheckReturnValue default ReactionPaginationAction retrieveReactionUsersById(@Nonnull String messageId, @Nonnull Emoji emoji) { - checkCanAccessChannel(); checkCanRemoveReactions(); return MessageChannelUnion.super.retrieveReactionUsersById(messageId, emoji); } @@ -314,7 +294,6 @@ default ReactionPaginationAction retrieveReactionUsersById(@Nonnull String messa @CheckReturnValue default RestAction pinMessageById(@Nonnull String messageId) { - checkCanAccessChannel(); checkCanControlMessagePins(); return MessageChannelUnion.super.pinMessageById(messageId); } @@ -323,7 +302,6 @@ default RestAction pinMessageById(@Nonnull String messageId) @CheckReturnValue default RestAction unpinMessageById(@Nonnull String messageId) { - checkCanAccessChannel(); checkCanControlMessagePins(); return MessageChannelUnion.super.unpinMessageById(messageId); } @@ -332,7 +310,6 @@ default RestAction unpinMessageById(@Nonnull String messageId) @CheckReturnValue default RestAction> retrievePinnedMessages() { - checkCanAccessChannel(); return MessageChannelUnion.super.retrievePinnedMessages(); } @@ -340,7 +317,6 @@ default RestAction> retrievePinnedMessages() @CheckReturnValue default MessageEditAction editMessageById(@Nonnull String messageId, @Nonnull CharSequence newContent) { - checkCanAccessChannel(); checkCanSendMessage(); return MessageChannelUnion.super.editMessageById(messageId, newContent); } @@ -349,7 +325,6 @@ default MessageEditAction editMessageById(@Nonnull String messageId, @Nonnull Ch @CheckReturnValue default MessageEditAction editMessageById(@Nonnull String messageId, @Nonnull MessageEditData data) { - checkCanAccessChannel(); checkCanSendMessage(); return MessageChannelUnion.super.editMessageById(messageId, data); } @@ -359,7 +334,6 @@ default MessageEditAction editMessageById(@Nonnull String messageId, @Nonnull Me @CheckReturnValue default MessageEditAction editMessageEmbedsById(@Nonnull String messageId, @Nonnull Collection newEmbeds) { - checkCanAccessChannel(); checkCanSendMessage(); checkCanSendMessageEmbeds(); return MessageChannelUnion.super.editMessageEmbedsById(messageId, newEmbeds); @@ -369,7 +343,6 @@ default MessageEditAction editMessageEmbedsById(@Nonnull String messageId, @Nonn @CheckReturnValue default MessageEditAction editMessageComponentsById(@Nonnull String messageId, @Nonnull Collection components) { - checkCanAccessChannel(); checkCanSendMessage(); return MessageChannelUnion.super.editMessageComponentsById(messageId, components); } @@ -378,7 +351,6 @@ default MessageEditAction editMessageComponentsById(@Nonnull String messageId, @ @Override default MessageEditAction editMessageAttachmentsById(@Nonnull String messageId, @Nonnull Collection attachments) { - checkCanAccessChannel(); checkCanSendMessage(); return MessageChannelUnion.super.editMessageAttachmentsById(messageId, attachments); } @@ -387,7 +359,6 @@ default MessageEditAction editMessageAttachmentsById(@Nonnull String messageId, T setLatestMessageIdLong(long latestMessageId); // ---- Mixin Hooks ---- - void checkCanAccessChannel(); void checkCanSendMessage(); void checkCanSendMessageEmbeds(); void checkCanSendFiles(); From 00512620611afbaf314800045e4c03c8ccf34936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Spie=C3=9F?= Date: Sun, 12 May 2024 13:07:16 +0200 Subject: [PATCH 2/2] Move checkCanAccess to ChannelMixin --- .../entities/channel/concrete/PrivateChannelImpl.java | 5 +++-- .../internal/entities/channel/mixin/ChannelMixin.java | 3 +++ .../mixin/middleman/GuildMessageChannelMixin.java | 4 ++++ .../channel/mixin/middleman/MessageChannelMixin.java | 9 ++++++++- 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/dv8tion/jda/internal/entities/channel/concrete/PrivateChannelImpl.java b/src/main/java/net/dv8tion/jda/internal/entities/channel/concrete/PrivateChannelImpl.java index cc8d32508a..fa303abf98 100644 --- a/src/main/java/net/dv8tion/jda/internal/entities/channel/concrete/PrivateChannelImpl.java +++ b/src/main/java/net/dv8tion/jda/internal/entities/channel/concrete/PrivateChannelImpl.java @@ -114,6 +114,9 @@ public boolean canTalk() return user == null || !user.isBot(); } + @Override + public void checkCanAccess() {} + @Override public void checkCanSendMessage() { checkBot(); @@ -155,8 +158,6 @@ public PrivateChannelImpl setLatestMessageIdLong(long latestMessageId) return this; } - // -- Object -- - @Override public int hashCode() { diff --git a/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/ChannelMixin.java b/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/ChannelMixin.java index 7f24ef5e51..ea68d8dfdd 100644 --- a/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/ChannelMixin.java +++ b/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/ChannelMixin.java @@ -42,4 +42,7 @@ default RestAction delete() // ---- State Accessors ---- T setName(String name); + + // ---- Hooks ---- + void checkCanAccess(); } diff --git a/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/GuildMessageChannelMixin.java b/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/GuildMessageChannelMixin.java index 0314fb724e..c93ac2f481 100644 --- a/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/GuildMessageChannelMixin.java +++ b/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/GuildMessageChannelMixin.java @@ -46,6 +46,7 @@ public interface GuildMessageChannelMixin> @CheckReturnValue default RestAction deleteMessagesByIds(@Nonnull Collection messageIds) { + checkCanAccess(); checkPermission(Permission.MESSAGE_MANAGE, "Must have MESSAGE_MANAGE in order to bulk delete messages in this channel regardless of author."); if (messageIds.size() < 2 || messageIds.size() > 100) @@ -66,6 +67,7 @@ default RestAction removeReactionById(@Nonnull String messageId, @Nonnull Checks.notNull(emoji, "Emoji"); Checks.notNull(user, "User"); + checkCanAccess(); if (!getJDA().getSelfUser().equals(user)) checkPermission(Permission.MESSAGE_MANAGE); @@ -85,6 +87,7 @@ default RestAction clearReactionsById(@Nonnull String messageId) { Checks.isSnowflake(messageId, "Message ID"); + checkCanAccess(); checkPermission(Permission.MESSAGE_MANAGE); final Route.CompiledRoute route = Route.Messages.REMOVE_ALL_REACTIONS.compile(getId(), messageId); @@ -98,6 +101,7 @@ default RestAction clearReactionsById(@Nonnull String messageId, @Nonnull Checks.notNull(messageId, "Message ID"); Checks.notNull(emoji, "Emoji"); + checkCanAccess(); checkPermission(Permission.MESSAGE_MANAGE); Route.CompiledRoute route = Route.Messages.CLEAR_EMOJI_REACTIONS.compile(getId(), messageId, emoji.getAsReactionCode()); diff --git a/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/MessageChannelMixin.java b/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/MessageChannelMixin.java index 28e86c12eb..3c80eff1c9 100644 --- a/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/MessageChannelMixin.java +++ b/src/main/java/net/dv8tion/jda/internal/entities/channel/mixin/middleman/MessageChannelMixin.java @@ -41,6 +41,7 @@ import net.dv8tion.jda.api.utils.messages.MessageCreateData; import net.dv8tion.jda.api.utils.messages.MessageEditData; import net.dv8tion.jda.api.utils.messages.MessagePollData; +import net.dv8tion.jda.internal.entities.channel.mixin.ChannelMixin; import net.dv8tion.jda.internal.requests.RestActionImpl; import org.jetbrains.annotations.NotNull; @@ -51,12 +52,14 @@ public interface MessageChannelMixin> extends MessageChannel, - MessageChannelUnion + MessageChannelUnion, + ChannelMixin { // ---- Default implementations of interface ---- @Nonnull default List> purgeMessages(@Nonnull List messages) { + checkCanAccess(); if (messages == null || messages.isEmpty()) return Collections.emptyList(); @@ -80,6 +83,7 @@ default List> purgeMessages(@Nonnull List> purgeMessagesById(@Nonnull long... messageIds) { + checkCanAccess(); if (messageIds == null || messageIds.length == 0) return Collections.emptyList(); @@ -207,6 +211,7 @@ default RestAction retrieveMessageById(@Nonnull String messageId) @CheckReturnValue default AuditableRestAction deleteMessageById(@Nonnull String messageId) { + checkCanAccess(); //We don't know if this is a Message sent by us or another user, so we can't run checks for Permission.MESSAGE_MANAGE return MessageChannelUnion.super.deleteMessageById(messageId); } @@ -263,6 +268,7 @@ default MessageHistory.MessageRetrieveAction getHistoryFromBeginning(int limit) @CheckReturnValue default RestAction sendTyping() { + checkCanAccess(); return MessageChannelUnion.super.sendTyping(); } @@ -310,6 +316,7 @@ default RestAction unpinMessageById(@Nonnull String messageId) @CheckReturnValue default RestAction> retrievePinnedMessages() { + checkCanAccess(); return MessageChannelUnion.super.retrievePinnedMessages(); }