stream_data: Use can_add_subscribers_group to check permissions.

web/src/stream_data.ts

@@ -542,12 +542,21 @@ export function can_view_subscribers(sub: StreamSubscription): boolean {
 }
 
 export function can_subscribe_others(sub: StreamSubscription): boolean {
-    // User can add other users to stream if stream is public or user is subscribed to stream
+    if (sub.invite_only && !sub.subscribed) {
-    // and realm level setting allows user to add subscribers.
+        return false;
-    return (
+    }
-        !current_user.is_guest &&
+
-        (!sub.invite_only || sub.subscribed) &&
+    if (settings_data.can_subscribe_others_to_all_streams()) {
-        settings_data.can_subscribe_others_to_all_streams()
+        return true;
+    }
+
+    if (can_change_permissions(sub)) {
+        return true;
+    }
+
+    return user_groups.is_user_in_setting_group(
+        sub.can_add_subscribers_group,
+        people.my_current_user_id(),
     );
 }
 

web/tests/stream_data.test.cjs

@@ -322,6 +322,9 @@ test("get_streams_for_user", ({override}) => {
         is_muted: true,
         invite_only: true,
         history_public_to_subscribers: true,
+        can_remove_subscribers_group: admins_group.id,
+        can_add_subscribers_group: admins_group.id,
+        can_administer_channel_group: admins_group.id,
     };
     const social = {
         color: "red",
@@ -330,6 +333,9 @@ test("get_streams_for_user", ({override}) => {
         is_muted: false,
         invite_only: false,
         history_public_to_subscribers: false,
+        can_remove_subscribers_group: admins_group.id,
+        can_add_subscribers_group: admins_group.id,
+        can_administer_channel_group: admins_group.id,
     };
     const test = {
         color: "yellow",
@@ -337,6 +343,9 @@ test("get_streams_for_user", ({override}) => {
         stream_id: 3,
         is_muted: true,
         invite_only: true,
+        can_remove_subscribers_group: admins_group.id,
+        can_add_subscribers_group: admins_group.id,
+        can_administer_channel_group: admins_group.id,
     };
     const world = {
         color: "blue",
@@ -345,6 +354,9 @@ test("get_streams_for_user", ({override}) => {
         is_muted: false,
         invite_only: false,
         history_public_to_subscribers: false,
+        can_remove_subscribers_group: admins_group.id,
+        can_add_subscribers_group: admins_group.id,
+        can_administer_channel_group: admins_group.id,
     };
     const errors = {
         color: "green",
@@ -353,6 +365,9 @@ test("get_streams_for_user", ({override}) => {
         is_muted: false,
         invite_only: false,
         history_public_to_subscribers: false,
+        can_remove_subscribers_group: admins_group.id,
+        can_add_subscribers_group: admins_group.id,
+        can_administer_channel_group: admins_group.id,
     };
     const subs = [denmark, social, test, world, errors];
     for (const sub of subs) {
@@ -380,11 +395,16 @@ test("get_streams_for_user", ({override}) => {
         social,
     ]);
     assert.deepEqual(stream_data.get_streams_for_user(test_user.user_id).can_subscribe, []);
-    // Verify that administrator cannot subscribe if they are not part
+    // Administrator is not part of the realm_can_add_subscribers_group
-    // of the appropriate group.
+    // or the stream level can_add_subscribers_group. But users with
+    // the permission to administer a channel can also subscribe other
+    // users. Admins can administer all channels they have access to.
     override(current_user, "is_admin", true);
     assert.equal(user_groups.is_user_in_group(students.id, current_user.user_id), false);
-    assert.deepEqual(stream_data.get_streams_for_user(test_user.user_id).can_subscribe, []);
+    assert.deepEqual(stream_data.get_streams_for_user(test_user.user_id).can_subscribe, [
+        world,
+        errors,
+    ]);
 
     override(realm, "realm_can_add_subscribers_group", everyone_group.id);
     assert.deepEqual(stream_data.get_streams_for_user(test_user.user_id).can_subscribe, [
@@ -435,6 +455,7 @@ test("admin_options", ({override}) => {
             invite_only: false,
             can_remove_subscribers_group: admins_group.id,
             can_administer_channel_group,
+            can_add_subscribers_group: admins_group.id,
             date_created: 1691057093,
             creator_id: null,
         };
@@ -519,6 +540,7 @@ test("stream_settings", ({override}) => {
         invite_only: false,
         can_remove_subscribers_group: admins_group.id,
         can_administer_channel_group: nobody_group.id,
+        can_add_subscribers_group: admins_group.id,
         date_created: 1691057093,
         creator_id: null,
     };
@@ -531,6 +553,7 @@ test("stream_settings", ({override}) => {
         invite_only: false,
         can_remove_subscribers_group: admins_group.id,
         can_administer_channel_group: nobody_group.id,
+        can_add_subscribers_group: admins_group.id,
         date_created: 1691057093,
         creator_id: null,
     };
@@ -545,6 +568,7 @@ test("stream_settings", ({override}) => {
         message_retention_days: 10,
         can_remove_subscribers_group: admins_group.id,
         can_administer_channel_group: nobody_group.id,
+        can_add_subscribers_group: admins_group.id,
         date_created: 1691057093,
         creator_id: null,
     };
@@ -1246,6 +1270,58 @@ test("can_unsubscribe_others", ({override}) => {
     assert.equal(stream_data.can_unsubscribe_others(sub), false);
 });
 
+test("can_subscribe_others", ({override}) => {
+    override(realm, "realm_can_add_subscribers_group", admins_group.id);
+    const sub = {
+        name: "Denmark",
+        subscribed: true,
+        color: "red",
+        stream_id: 1,
+        can_add_subscribers_group: admins_group.id,
+        can_administer_channel_group: nobody_group.id,
+        can_remove_subscribers_group: admins_group.id,
+    };
+    stream_data.add_sub(sub);
+
+    people.initialize_current_user(admin_user_id);
+    assert.equal(stream_data.can_subscribe_others(sub), true);
+    people.initialize_current_user(moderator_user_id);
+    assert.equal(stream_data.can_subscribe_others(sub), false);
+
+    sub.can_add_subscribers_group = moderators_group.id;
+    people.initialize_current_user(admin_user_id);
+    assert.equal(stream_data.can_subscribe_others(sub), true);
+    people.initialize_current_user(moderator_user_id);
+    assert.equal(stream_data.can_subscribe_others(sub), true);
+    people.initialize_current_user(test_user.user_id);
+    assert.equal(stream_data.can_subscribe_others(sub), false);
+
+    sub.can_add_subscribers_group = everyone_group.id;
+    people.initialize_current_user(admin_user_id);
+    assert.equal(stream_data.can_subscribe_others(sub), true);
+    people.initialize_current_user(moderator_user_id);
+    assert.equal(stream_data.can_subscribe_others(sub), true);
+    people.initialize_current_user(test_user.user_id);
+    assert.equal(stream_data.can_subscribe_others(sub), true);
+
+    // With the setting set to user defined group not including admin,
+    // admin can still subscribe others.
+    sub.can_add_subscribers_group = students.id;
+    override(current_user, "is_admin", true);
+    people.initialize_current_user(admin_user_id);
+    assert.equal(stream_data.can_subscribe_others(sub), true);
+    override(current_user, "is_admin", false);
+    people.initialize_current_user(moderator_user_id);
+    assert.equal(stream_data.can_subscribe_others(sub), false);
+    people.initialize_current_user(test_user.user_id);
+    assert.equal(stream_data.can_subscribe_others(sub), true);
+
+    sub.can_remove_subscribers_group = everyone_group.id;
+    sub.subscribed = false;
+    sub.invite_only = true;
+    assert.equal(stream_data.can_subscribe_others(sub), false);
+});
+
 test("options for dropdown widget", () => {
     const denmark = {
         subscribed: true,

web/tests/stream_settings_ui.test.cjs

@@ -79,6 +79,7 @@ run_test("redraw_left_panel", ({override, mock_template}) => {
         color: "red",
         can_administer_channel_group: nobody_group.id,
         can_remove_subscribers_group: admins_group.id,
+        can_add_subscribers_group: admins_group.id,
         date_created: 1691057093,
         creator_id: null,
     };
@@ -93,6 +94,7 @@ run_test("redraw_left_panel", ({override, mock_template}) => {
         color: "red",
         can_administer_channel_group: nobody_group.id,
         can_remove_subscribers_group: admins_group.id,
+        can_add_subscribers_group: admins_group.id,
         date_created: 1691057093,
         creator_id: null,
     };
@@ -107,6 +109,7 @@ run_test("redraw_left_panel", ({override, mock_template}) => {
         color: "red",
         can_remove_subscribers_group: admins_group.id,
         can_administer_channel_group: nobody_group.id,
+        can_add_subscribers_group: admins_group.id,
         date_created: 1691057093,
         creator_id: null,
     };
@@ -121,6 +124,7 @@ run_test("redraw_left_panel", ({override, mock_template}) => {
         color: "red",
         can_administer_channel_group: nobody_group.id,
         can_remove_subscribers_group: admins_group.id,
+        can_add_subscribers_group: admins_group.id,
         date_created: 1691057093,
         creator_id: null,
     };
@@ -135,6 +139,7 @@ run_test("redraw_left_panel", ({override, mock_template}) => {
         color: "red",
         can_administer_channel_group: nobody_group.id,
         can_remove_subscribers_group: admins_group.id,
+        can_add_subscribers_group: admins_group.id,
         date_created: 1691057093,
         creator_id: null,
     };
@@ -149,6 +154,7 @@ run_test("redraw_left_panel", ({override, mock_template}) => {
         color: "red",
         can_administer_channel_group: nobody_group.id,
         can_remove_subscribers_group: admins_group.id,
+        can_add_subscribers_group: admins_group.id,
         date_created: 1691057093,
         creator_id: null,
     };
@@ -163,6 +169,7 @@ run_test("redraw_left_panel", ({override, mock_template}) => {
         color: "red",
         can_administer_channel_group: nobody_group.id,
         can_remove_subscribers_group: admins_group.id,
+        can_add_subscribers_group: admins_group.id,
         date_created: 1691057093,
         creator_id: null,
     };
@@ -177,6 +184,7 @@ run_test("redraw_left_panel", ({override, mock_template}) => {
         color: "red",
         can_administer_channel_group: nobody_group.id,
         can_remove_subscribers_group: admins_group.id,
+        can_add_subscribers_group: admins_group.id,
         date_created: 1691057093,
         creator_id: null,
     };