FIX: Ensure moderators_manage_categories_and_groups is respected (#18884)

Currently, moderators are able to set primary group for users
irrespective of the of the `moderators_manage_categories_and_groups` site
setting value.

This change updates Guardian implementation to honour it.

app/controllers/admin/groups_controller.rb

@@ -111,7 +111,7 @@ class Admin::GroupsController < Admin::StaffController
     raise Discourse::NotFound unless group
 
     users = User.where(username: group_params[:usernames].split(","))
-    users.each { |user| guardian.ensure_can_change_primary_group!(user) }
+    users.each { |user| guardian.ensure_can_change_primary_group!(user, group) }
     users.update_all(primary_group_id: params[:primary] == "true" ? group.id : nil)
 
     render json: success_json

app/controllers/admin/users_controller.rb

@@ -241,11 +241,11 @@ class Admin::UsersController < Admin::StaffController
   end
 
   def primary_group
-    guardian.ensure_can_change_primary_group!(@user)
-
     if params[:primary_group_id].present?
       primary_group_id = params[:primary_group_id].to_i
       if group = Group.find(primary_group_id)
+        guardian.ensure_can_change_primary_group!(@user, group)
+
         if group.user_ids.include?(@user.id)
           @user.primary_group_id = primary_group_id
         end

lib/guardian.rb

@@ -359,8 +359,8 @@ class Guardian
     flair_icon.present? || flair_upload_id.present?
   end
 
-  def can_change_primary_group?(user)
+  def can_change_primary_group?(user, group)
-    user && is_staff?
+    user && can_edit_group?(group)
   end
 
   def can_change_trust_level?(user)

spec/lib/guardian_spec.rb

@@ -2800,6 +2800,40 @@ RSpec.describe Guardian do
     end
   end
 
+  describe "#can_change_primary_group?" do
+    it "returns false without a logged in user" do
+      expect(Guardian.new(nil).can_change_primary_group?(user, group)).to eq(false)
+    end
+
+    it "returns false for regular users" do
+      expect(Guardian.new(user).can_change_primary_group?(user, group)).to eq(false)
+    end
+
+    it "returns true for admins" do
+      expect(Guardian.new(admin).can_change_primary_group?(user, group)).to eq(true)
+    end
+
+    context "when moderators_manage_categories_and_groups site setting is enabled" do
+      before do
+        SiteSetting.moderators_manage_categories_and_groups = true
+      end
+
+      it "returns true for moderators" do
+        expect(Guardian.new(moderator).can_change_primary_group?(user, group)).to eq(true)
+      end
+    end
+
+    context "when moderators_manage_categories_and_groups site setting is disabled" do
+      before do
+        SiteSetting.moderators_manage_categories_and_groups = false
+      end
+
+      it "returns false for moderators" do
+        expect(Guardian.new(moderator).can_change_primary_group?(user, group)).to eq(false)
+      end
+    end
+  end
+
   describe 'can_change_trust_level?' do
 
     it 'is false without a logged in user' do

spec/requests/admin/groups_controller_spec.rb

@@ -472,7 +472,7 @@ RSpec.describe Admin::GroupsController do
           SiteSetting.moderators_manage_categories_and_groups = false
         end
 
-        it "sets multiple primary users" do
+        it "prevents setting of primary group with a 403 response" do
           user2.update!(primary_group_id: group.id)
 
           put "/admin/groups/#{group.id}/primary.json", params: {
@@ -480,8 +480,9 @@ RSpec.describe Admin::GroupsController do
             primary: "true"
           }
 
-          expect(response.status).to eq(200)
+          expect(response.status).to eq(403)
-          expect(User.where(primary_group_id: group.id).size).to eq(3)
+          expect(response.parsed_body["errors"]).to include(I18n.t("invalid_access"))
+          expect(User.where(primary_group_id: group.id).size).to eq(1)
         end
       end
     end

spec/requests/admin/users_controller_spec.rb

@@ -1052,7 +1052,31 @@ RSpec.describe Admin::UsersController do
     context "when logged in as a moderator" do
       before { sign_in(moderator) }
 
-      include_examples "primary group updates possible"
+      context "when moderators_manage_categories_and_groups site setting is enabled" do
+        before do
+          SiteSetting.moderators_manage_categories_and_groups = true
+        end
+
+        include_examples "primary group updates possible"
+      end
+
+      context "when moderators_manage_categories_and_groups site setting is disabled" do
+        before do
+          SiteSetting.moderators_manage_categories_and_groups = false
+        end
+
+        it "prevents setting primary group with a 403 response" do
+          group.add(another_user)
+          put "/admin/users/#{another_user.id}/primary_group.json", params: {
+            primary_group_id: group.id
+          }
+
+          expect(response.status).to eq(403)
+          expect(response.parsed_body["errors"]).to include(I18n.t("invalid_access"))
+          another_user.reload
+          expect(another_user.primary_group_id).to eq(nil)
+        end
+      end
     end
 
     context "when logged in as a non-staff user" do