From a2486de04502226bc15f5495b61a0aebbca0d835 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 4 Aug 2023 09:58:10 -0700 Subject: [PATCH] Don't expose channel admin actions in UI if user isn't admin --- crates/client/src/channel_store.rs | 5 +- crates/collab/src/rpc.rs | 29 ++++--- crates/collab/src/tests/channel_tests.rs | 105 +++++++++++++++++++---- crates/collab_ui/src/collab_panel.rs | 28 +++--- 4 files changed, 123 insertions(+), 44 deletions(-) diff --git a/crates/client/src/channel_store.rs b/crates/client/src/channel_store.rs index ee04865e50..c04b123acf 100644 --- a/crates/client/src/channel_store.rs +++ b/crates/client/src/channel_store.rs @@ -263,13 +263,14 @@ impl ChannelStore { if let Some(parent_id) = channel.parent_id { if let Some(ix) = self.channels.iter().position(|c| c.id == parent_id) { - let depth = self.channels[ix].depth + 1; + let parent_channel = &self.channels[ix]; + let depth = parent_channel.depth + 1; self.channels.insert( ix + 1, Arc::new(Channel { id: channel.id, name: channel.name, - user_is_admin: channel.user_is_admin, + user_is_admin: channel.user_is_admin || parent_channel.user_is_admin, parent_id: Some(parent_id), depth, }), diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 31b0b2280a..6893c4bde4 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -2209,7 +2209,7 @@ async fn invite_channel_member( .await? .ok_or_else(|| anyhow!("channel not found"))?; let invitee_id = UserId::from_proto(request.user_id); - db.invite_channel_member(channel_id, invitee_id, session.user_id, false) + db.invite_channel_member(channel_id, invitee_id, session.user_id, request.admin) .await?; let mut update = proto::UpdateChannels::default(); @@ -2268,22 +2268,29 @@ async fn respond_to_channel_invite( let channel_id = ChannelId::from_proto(request.channel_id); db.respond_to_channel_invite(channel_id, session.user_id, request.accept) .await?; - let channel = db - .get_channel(channel_id, session.user_id) - .await? - .ok_or_else(|| anyhow!("no such channel"))?; let mut update = proto::UpdateChannels::default(); update .remove_channel_invitations .push(channel_id.to_proto()); if request.accept { - update.channels.push(proto::Channel { - id: channel.id.to_proto(), - name: channel.name, - user_is_admin: channel.user_is_admin, - parent_id: None, - }); + let (channels, participants) = db.get_channels_for_user(session.user_id).await?; + update + .channels + .extend(channels.into_iter().map(|channel| proto::Channel { + id: channel.id.to_proto(), + name: channel.name, + user_is_admin: channel.user_is_admin, + parent_id: channel.parent_id.map(ChannelId::to_proto), + })); + update + .channel_participants + .extend(participants.into_iter().map(|(channel_id, user_ids)| { + proto::ChannelParticipants { + channel_id: channel_id.to_proto(), + participant_user_ids: user_ids.into_iter().map(UserId::to_proto).collect(), + } + })); } session.peer.send(session.connection_id, update)?; response.send(proto::Ack {})?; diff --git a/crates/collab/src/tests/channel_tests.rs b/crates/collab/src/tests/channel_tests.rs index abaedb52a8..43e5a296c4 100644 --- a/crates/collab/src/tests/channel_tests.rs +++ b/crates/collab/src/tests/channel_tests.rs @@ -23,18 +23,34 @@ async fn test_basic_channels( }) .await .unwrap(); + let channel_b_id = client_a + .channel_store() + .update(cx_a, |channel_store, _| { + channel_store.create_channel("channel-b", Some(channel_a_id)) + }) + .await + .unwrap(); deterministic.run_until_parked(); client_a.channel_store().read_with(cx_a, |channels, _| { assert_eq!( channels.channels(), - &[Arc::new(Channel { - id: channel_a_id, - name: "channel-a".to_string(), - parent_id: None, - user_is_admin: true, - depth: 0, - })] + &[ + Arc::new(Channel { + id: channel_a_id, + name: "channel-a".to_string(), + parent_id: None, + user_is_admin: true, + depth: 0, + }), + Arc::new(Channel { + id: channel_b_id, + name: "channel-b".to_string(), + parent_id: Some(channel_a_id), + user_is_admin: true, + depth: 1, + }) + ] ) }); @@ -48,7 +64,7 @@ async fn test_basic_channels( .update(cx_a, |store, cx| { assert!(!store.has_pending_channel_invite(channel_a_id, client_b.user_id().unwrap())); - let invite = store.invite_member(channel_a_id, client_b.user_id().unwrap(), false, cx); + let invite = store.invite_member(channel_a_id, client_b.user_id().unwrap(), true, cx); // Make sure we're synchronously storing the pending invite assert!(store.has_pending_channel_invite(channel_a_id, client_b.user_id().unwrap())); @@ -57,9 +73,8 @@ async fn test_basic_channels( .await .unwrap(); - // Wait for client b to see the invitation + // Client A sees that B has been invited. deterministic.run_until_parked(); - client_b.channel_store().read_with(cx_b, |channels, _| { assert_eq!( channels.channel_invitations(), @@ -69,7 +84,7 @@ async fn test_basic_channels( parent_id: None, user_is_admin: false, depth: 0, - })] + }),] ) }); let members = client_a @@ -94,7 +109,7 @@ async fn test_basic_channels( ], ); - // Client B now sees that they are a member channel A. + // Client B accepts the invitation. client_b .channel_store() .update(cx_b, |channels, _| { @@ -102,17 +117,69 @@ async fn test_basic_channels( }) .await .unwrap(); + + // Client B now sees that they are a member of channel A and its existing + // subchannels. Their admin priveleges extend to subchannels of channel A. + deterministic.run_until_parked(); client_b.channel_store().read_with(cx_b, |channels, _| { assert_eq!(channels.channel_invitations(), &[]); assert_eq!( channels.channels(), - &[Arc::new(Channel { - id: channel_a_id, - name: "channel-a".to_string(), - parent_id: None, - user_is_admin: false, - depth: 0, - })] + &[ + Arc::new(Channel { + id: channel_a_id, + name: "channel-a".to_string(), + parent_id: None, + user_is_admin: true, + depth: 0, + }), + Arc::new(Channel { + id: channel_b_id, + name: "channel-b".to_string(), + parent_id: Some(channel_a_id), + user_is_admin: true, + depth: 1, + }) + ] + ) + }); + + let channel_c_id = client_a + .channel_store() + .update(cx_a, |channel_store, _| { + channel_store.create_channel("channel-c", Some(channel_a_id)) + }) + .await + .unwrap(); + + // TODO - ensure sibling channels are sorted in a stable way + deterministic.run_until_parked(); + client_b.channel_store().read_with(cx_b, |channels, _| { + assert_eq!( + channels.channels(), + &[ + Arc::new(Channel { + id: channel_a_id, + name: "channel-a".to_string(), + parent_id: None, + user_is_admin: true, + depth: 0, + }), + Arc::new(Channel { + id: channel_c_id, + name: "channel-c".to_string(), + parent_id: Some(channel_a_id), + user_is_admin: true, + depth: 1, + }), + Arc::new(Channel { + id: channel_b_id, + name: "channel-b".to_string(), + parent_id: Some(channel_a_id), + user_is_admin: true, + depth: 1, + }), + ] ) }); diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 771927c8ac..df27ea5005 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -1509,18 +1509,22 @@ impl CollabPanel { channel_id: u64, cx: &mut ViewContext, ) { - self.context_menu.update(cx, |context_menu, cx| { - context_menu.show( - position, - gpui::elements::AnchorCorner::BottomLeft, - vec![ - ContextMenuItem::action("New Channel", NewChannel { channel_id }), - ContextMenuItem::action("Remove Channel", RemoveChannel { channel_id }), - ContextMenuItem::action("Add member", AddMember { channel_id }), - ], - cx, - ); - }); + if let Some(channel) = self.channel_store.read(cx).channel_for_id(channel_id) { + if channel.user_is_admin { + self.context_menu.update(cx, |context_menu, cx| { + context_menu.show( + position, + gpui::elements::AnchorCorner::BottomLeft, + vec![ + ContextMenuItem::action("New Channel", NewChannel { channel_id }), + ContextMenuItem::action("Remove Channel", RemoveChannel { channel_id }), + ContextMenuItem::action("Add member", AddMember { channel_id }), + ], + cx, + ); + }); + } + } } fn cancel(&mut self, _: &Cancel, cx: &mut ViewContext) {