diff --git a/CHANGELOG.md b/CHANGELOG.md index 11c32fd..32fef5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## v0.2.7 +- Fix some wrong responses to admin commands +- Remove automatic announcements from some admin commands +- Send no reply to unauthorized admin commands +- Fix swapped `/opengroup` and `/closegroup` descriptions in help + ## v0.2.6 - Allow boosting group hashtags when they are in a reply, except when it is private/DM or contains actionable commands diff --git a/Cargo.lock b/Cargo.lock index 744c098..dc05d2d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -328,7 +328,7 @@ checksum = "e88a8acf291dafb59c2d96e8f59828f3838bb1a70398823ade51a84de6a6deed" [[package]] name = "fedigroups" -version = "0.2.6" +version = "0.2.7" dependencies = [ "anyhow", "clap", diff --git a/Cargo.toml b/Cargo.toml index 2bf3eb8..30d79f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "fedigroups" -version = "0.2.6" +version = "0.2.7" authors = ["Ondřej Hruška "] edition = "2018" publish = false diff --git a/README.md b/README.md index 761c69c..ab602da 100644 --- a/README.md +++ b/README.md @@ -175,8 +175,8 @@ For group hashtags to work, the group user must follow all its members; otherwis - `/unban domain.tld` - lift a server ban - `/op user@domain` (alias `/admin`) - grant admin rights to a user - `/deop user@domain` (alias `/deadmin`) - revoke admin rights -- `/opengroup` - make the group member-only -- `/closegroup` - make the group public-access +- `/closegroup` - make the group member-only +- `/opengroup` - make the group public-access - `/add user@domain` (alias `/follow`) - add a member - `/remove user@domain` (alias `/remove`) - remove a member - `/add #hashtag` (alias `/follow`) - add a hashtag to the group diff --git a/src/group_handler/handle_mention.rs b/src/group_handler/handle_mention.rs index 3a7cf19..a47eef1 100644 --- a/src/group_handler/handle_mention.rs +++ b/src/group_handler/handle_mention.rs @@ -134,11 +134,11 @@ impl<'a> ProcessMention<'a> { } fn add_reply(&mut self, line: impl AsRef) { - self.replies.push(line.as_ref().trim().to_string()) + self.replies.push(line.as_ref().trim_matches(' ').to_string()) } fn add_announcement<'t>(&mut self, line: impl AsRef) { - self.announcements.push(line.as_ref().trim().to_string()) + self.announcements.push(line.as_ref().trim_matches(' ').to_string()) } async fn handle(mut self) -> Result<(), GroupError> { @@ -196,7 +196,7 @@ impl<'a> ProcessMention<'a> { self.cmd_remove_tag(tag).await; } StatusCommand::GrantAdmin(u) => { - self.cmd_grant_member(&u).await + self.cmd_grant_admin(&u).await .log_error("Error handling grant-admin cmd"); } StatusCommand::RemoveAdmin(u) => { @@ -292,8 +292,9 @@ impl<'a> ProcessMention<'a> { // tokio::time::sleep(DELAY_BEFORE_ACTION).await; self.reblog_status().await; } else { - self.add_reply("You are not allowed to post to this group"); + warn!("User @{} can't post to group!", self.status_acct); } + // Otherwise, don't react } else { debug!("Not OP, ignore mention"); } @@ -308,7 +309,7 @@ impl<'a> ProcessMention<'a> { if self.can_write { self.do_boost_prev_post = self.status.in_reply_to_id.is_some(); } else { - self.add_reply("You are not allowed to share to this group"); + warn!("User @{} can't share to group!", self.status_acct); } } @@ -321,7 +322,7 @@ impl<'a> ProcessMention<'a> { info!("Deleting group post #{}", parent_status_id); self.client.delete_status(parent_status_id).await?; } else { - warn!("Only admin can delete announcements."); + warn!("Only admin can delete posts made by the group user"); } } else { if self.is_admin || parent_account_id == &self.status_user_id { @@ -329,7 +330,9 @@ impl<'a> ProcessMention<'a> { // User unboosting own post boosted by accident, or admin doing it self.client.unreblog(parent_status_id).await?; } else { - self.add_reply("You don't have rights to do that."); + warn!("Only the author and admins can undo reblogs"); + // XXX this means when someone /b's someone else's post to a group, + // they then can't reverse that (only admin or the post's author can) } } } @@ -351,9 +354,11 @@ impl<'a> ProcessMention<'a> { self.add_reply(format!("Failed to ban user {}: {}", u, e)); } } + } else { + self.add_reply(format!("No action, user {} is already banned", u)); } } else { - self.add_reply("Only admins can manage user bans"); + warn!("Ignore cmd, user not admin"); } Ok(()) } @@ -371,9 +376,11 @@ impl<'a> ProcessMention<'a> { unreachable!() } } + } else { + self.add_reply(format!("No action, user {} is not banned", u)); } } else { - self.add_reply("Only admins can manage user bans"); + warn!("Ignore cmd, user not admin"); } Ok(()) } @@ -383,16 +390,17 @@ impl<'a> ProcessMention<'a> { if !self.config.is_server_banned(s) { match self.config.ban_server(s, true) { Ok(_) => { - self.add_announcement(format!("Server \"{}\" has been banned.", s)); self.add_reply(format!("Server {} banned from group!", s)); } Err(e) => { self.add_reply(format!("Failed to ban server {}: {}", s, e)); } } + } else { + self.add_reply(format!("No action, server {} already banned", s)); } } else { - self.add_reply("Only admins can manage server bans"); + warn!("Ignore cmd, user not admin"); } } @@ -401,22 +409,24 @@ impl<'a> ProcessMention<'a> { if self.config.is_server_banned(s) { match self.config.ban_server(s, false) { Ok(_) => { - self.add_announcement(format!("Server \"{}\" has been un-banned.", s)); self.add_reply(format!("Server {} un-banned!", s)); } - Err(_) => { - unreachable!() + Err(e) => { + self.add_reply(format!("Unexpected error occured: {}", e)); } } + } else { + self.add_reply(format!("No action, server {} is not banned", s)); } } else { - self.add_reply("Only admins can manage server bans"); + warn!("Ignore cmd, user not admin"); } } async fn cmd_add_member(&mut self, user: &str) -> Result<(), GroupError> { let u = normalize_acct(user, &self.group_acct)?; if self.is_admin { + // Allow even if the user is already a member - that will trigger re-follow match self.config.set_member(&u, true) { Ok(_) => { self.add_reply(format!("User {} added to the group!", u)); @@ -428,7 +438,7 @@ impl<'a> ProcessMention<'a> { } } } else { - self.add_reply("Only admins can manage members"); + warn!("Ignore cmd, user not admin"); } Ok(()) } @@ -442,26 +452,26 @@ impl<'a> ProcessMention<'a> { self.unfollow_by_acct(&u).await .log_error("Failed to unfollow removed user"); } - Err(_) => { - unreachable!() + Err(e) => { + self.add_reply(format!("Unexpected error occured: {}", e)); } } } else { - self.add_reply("Only admins can manage members"); + warn!("Ignore cmd, user not admin"); } Ok(()) } async fn cmd_add_tag(&mut self, tag: String) { if self.is_admin { - if self.config.is_tag_followed(&tag) { + if !self.config.is_tag_followed(&tag) { + self.config.add_tag(&tag); self.add_reply(format!("Tag \"{}\" added to the group!", tag)); } else { - self.config.add_tag(&tag); - self.add_reply(format!("Tag \"{}\" was already in group!", tag)); + self.add_reply(format!("No action, \"{}\" is already a group tag", tag)); } } else { - self.add_reply("Only admins can manage group tags"); + warn!("Ignore cmd, user not admin"); } } @@ -471,14 +481,14 @@ impl<'a> ProcessMention<'a> { self.config.remove_tag(&tag); self.add_reply(format!("Tag \"{}\" removed from the group!", tag)); } else { - self.add_reply(format!("Tag \"{}\" was not in group!", tag)); + self.add_reply(format!("No action, \"{}\" is not a group tag", tag)); } } else { - self.add_reply("Only admins can manage group tags"); + warn!("Ignore cmd, user not admin"); } } - async fn cmd_grant_member(&mut self, user: &str) -> Result<(), GroupError> { + async fn cmd_grant_admin(&mut self, user: &str) -> Result<(), GroupError> { let u = normalize_acct(user, &self.group_acct)?; if self.is_admin { if !self.config.is_admin(&u) { @@ -488,7 +498,6 @@ impl<'a> ProcessMention<'a> { let _ = self.config.set_member(&u, true); self.add_reply(format!("User {} is now a group admin!", u)); - self.add_announcement(format!("User @{} can now manage this group!", u)); } Err(e) => { self.add_reply(format!( @@ -497,9 +506,11 @@ impl<'a> ProcessMention<'a> { )); } } + } else { + self.add_reply(format!("No action, \"{}\" is admin already", u)); } } else { - self.add_reply("Only admins can manage admins"); + warn!("Ignore cmd, user not admin"); } Ok(()) } @@ -511,15 +522,16 @@ impl<'a> ProcessMention<'a> { match self.config.set_admin(&u, false) { Ok(_) => { self.add_reply(format!("User {} is no longer a group admin!", u)); - self.add_announcement(format!("User @{} no longer manages this group.", u)); } Err(e) => { self.add_reply(format!("Failed to revoke {}'s group admin: {}", u, e)); } } + } else { + self.add_reply(format!("No action, user {} is not admin", u)); } } else { - self.add_reply("Only admins can manage admins"); + warn!("Ignore cmd, user not admin"); } Ok(()) } @@ -529,10 +541,11 @@ impl<'a> ProcessMention<'a> { if self.config.is_member_only() { self.config.set_member_only(false); self.add_reply("Group changed to open-access"); - self.add_announcement("This group is now open-access!"); + } else { + self.add_reply("No action, group is open-access already"); } } else { - self.add_reply("Only admins can set group options"); + warn!("Ignore cmd, user not admin"); } } @@ -541,33 +554,34 @@ impl<'a> ProcessMention<'a> { if !self.config.is_member_only() { self.config.set_member_only(true); self.add_reply("Group changed to member-only"); - self.add_announcement("This group is now member-only!"); + } else { + self.add_reply("No action, group is member-only already"); } } else { - self.add_reply("Only admins can set group options"); + warn!("Ignore cmd, user not admin"); } } async fn cmd_help(&mut self) { self.want_markdown = true; - if self.config.is_member_only() { - self.add_reply("This is a member-only group. "); - } else { - self.add_reply("This is a public-access group. "); - } - - if self.is_admin { - self.add_reply("*You are an admin.*"); + let membership_line = if self.is_admin { + "*You are an admin.*" } else if self.config.is_member(&self.status_acct) { - self.add_reply("*You are a member.*"); + "*You are a member.*" } else if self.config.is_member_only() { - self.add_reply("*You are not a member, ask one of the admins to add you.*"); + "*You are not a member, ask one of the admins to add you.*" + } else { + "*You are not a member, follow or use /join to join the group.*" + }; + + if self.config.is_member_only() { + self.add_reply(format!("This is a member-only group. {}", membership_line)); } else { - self.add_reply("*You are not a member, follow or use /join to join the group.*"); + self.add_reply(format!("This is a public-access group. {}", membership_line)); } - self.add_reply("\n\ + self.add_reply("\ To share a post, @ the group user or use a group hashtag.\n\ \n\ **Supported commands:**\n\ @@ -600,8 +614,8 @@ impl<'a> ProcessMention<'a> { `/unban x` - lift a ban\n\ `/admin user` - grant admin rights\n\ `/deadmin user` - revoke admin rights\n\ - `/opengroup` - make member-only\n\ - `/closegroup` - make public-access\n\ + `/closegroup` - make member-only\n\ + `/opengroup` - make public-access\n\ `/announce x` - make a public announcement"); } }