GRN2-xx: Admin actions are now dictated by the correct role permission (#1140)

* Admin actions are now dictated by the correct role permission

* Rspec fix

Co-authored-by: Jesus Federico <jesus@123it.ca>

app/assets/javascripts/room.js

@@ -162,7 +162,7 @@ function showUpdateRoom(target) {
   var modal = $(target)
   var update_path = modal.closest("#room-block").data("path")
   var settings_path = modal.data("settings-path")
-  $("#create-room-name").val(modal.closest("#room-block").find("#room-name-text").text())
+  $("#create-room-name").val(modal.closest("#room-block").find("#room-name-text").text().trim())
   $("#createRoomModal form").attr("action", update_path)
 
   //show all elements & their children with a update-only class

app/controllers/admins_controller.rb

@@ -309,7 +309,7 @@ class AdminsController < ApplicationController
   # Verifies that admin is an administrator of the user in the action
   def verify_admin_of_user
     redirect_to admins_path,
-      flash: { alert: I18n.t("administrator.flash.unauthorized") } unless current_user.admin_of?(@user)
+      flash: { alert: I18n.t("administrator.flash.unauthorized") } unless current_user.admin_of?(@user, "can_manage_users")
   end
 
   # Creates the invite if it doesn't exist, or updates the updated_at time if it does

app/controllers/concerns/joiner.rb

@@ -53,7 +53,7 @@ module Joiner
     if room_running?(@room.bbb_id) || @room.owned_by?(current_user) || room_settings["anyoneCanStart"]
 
       # Determine if the user needs to join as a moderator.
-      opts[:user_is_moderator] = @room.owned_by?(current_user) || room_settings["joinModerator"]
+      opts[:user_is_moderator] = @room.owned_by?(current_user) || room_settings["joinModerator"] || @shared_room
 
       opts[:require_moderator_approval] = room_settings["requireModeratorApproval"]
       opts[:mute_on_start] = room_settings["muteOnStart"]

app/controllers/rooms_controller.rb

@@ -93,7 +93,9 @@ class RoomsController < ApplicationController
     return redirect_to root_path,
       flash: { alert: I18n.t("administrator.site_settings.authentication.user-info") } if auth_required
 
-    unless @room.owned_by?(current_user) || room_shared_with_user
+    @shared_room = room_shared_with_user
+
+    unless @room.owned_by?(current_user) || @shared_room
       # Don't allow users to join unless they have a valid access code or the room doesn't have an access code
       if @room.access_code && !@room.access_code.empty? && @room.access_code != session[:access_code]
         return redirect_to room_path(room_uid: params[:room_uid]), flash: { alert: I18n.t("room.access_code_required") }
@@ -300,12 +302,13 @@ class RoomsController < ApplicationController
   def verify_room_ownership_or_admin_or_shared
     return redirect_to root_path unless @room.owned_by?(current_user) ||
                                         room_shared_with_user ||
-                                        current_user&.admin_of?(@room.owner)
+                                        current_user&.admin_of?(@room.owner, "can_manage_rooms_recordings")
   end
 
   # Ensure the user either owns the room or is an admin of the room owner
   def verify_room_ownership_or_admin
-    return redirect_to root_path if !@room.owned_by?(current_user) && !current_user&.admin_of?(@room.owner)
+    return redirect_to root_path if !@room.owned_by?(current_user) &&
+                                    !current_user&.admin_of?(@room.owner, "can_manage_rooms_recordings")
   end
 
   # Ensure the user owns the room or is allowed to start it

app/controllers/users_controller.rb

@@ -89,7 +89,7 @@ class UsersController < ApplicationController
       path = admins_path
     end
 
-    redirect_path = current_user.admin_of?(@user) ? path : profile
+    redirect_path = current_user.admin_of?(@user, "can_manage_users") ? path : profile
 
     if params[:setting] == "password"
       # Update the users password.
@@ -141,7 +141,7 @@ class UsersController < ApplicationController
     redirect_url = self_delete ? root_path : admin_path
 
     begin
-      if current_user && (self_delete || current_user.admin_of?(@user))
+      if current_user && (self_delete || current_user.admin_of?(@user, "can_manage_users"))
         # Permanently delete if the user is deleting themself
         perm_delete = self_delete || (params[:permanent].present? && params[:permanent] == "true")
 
@@ -216,6 +216,8 @@ class UsersController < ApplicationController
 
   # Checks that the user is allowed to edit this user
   def check_admin_of
-    redirect_to current_user.main_room if current_user && @user != current_user && !current_user.admin_of?(@user)
+    redirect_to current_user.main_room if current_user &&
+                                          @user != current_user &&
+                                          !current_user.admin_of?(@user, "can_manage_users")
   end
 end

app/models/ability.rb

@@ -27,19 +27,19 @@ class Ability
     else
       highest_role = user.highest_priority_role
       if highest_role.get_permission("can_edit_site_settings")
-        can [:index, :site_settings, :update_settings, :coloring, :registration_method], :admin
+        can [:site_settings, :update_settings, :coloring, :registration_method], :admin
       end
 
       if highest_role.get_permission("can_edit_roles")
-        can [:index, :roles, :new_role, :change_role_order, :update_role, :delete_role], :admin
+        can [:roles, :new_role, :change_role_order, :update_role, :delete_role], :admin
       end
 
       if highest_role.get_permission("can_manage_users")
-        can [:index, :roles, :edit_user, :promote, :demote, :ban_user, :unban_user,
+        can [:index, :edit_user, :promote, :demote, :ban_user, :unban_user,
              :approve, :invite, :reset, :undelete, :merge_user], :admin
       end
 
-      can [:index, :server_recordings, :server_rooms], :admin if highest_role.get_permission("can_manage_rooms_recordings")
+      can [:server_recordings, :server_rooms], :admin if highest_role.get_permission("can_manage_rooms_recordings")
 
       if !highest_role.get_permission("can_edit_site_settings") && !highest_role.get_permission("can_edit_roles") &&
          !highest_role.get_permission("can_manage_users") && !highest_role.get_permission("can_manage_rooms_recordings")

app/models/user.rb

@@ -163,17 +163,12 @@ class User < ApplicationRecord
     update_attributes(activation_digest: User.digest(activation_token))
   end
 
-  def admin_of?(user)
-    if Rails.configuration.loadbalanced_configuration
-      if has_role? :super_admin
-        id != user.id
-      else
-        highest_priority_role.get_permission("can_manage_users") && (id != user.id) && (provider == user.provider) &&
-          (!user.has_role? :super_admin)
-      end
-    else
-      (highest_priority_role.get_permission("can_manage_users") || (has_role? :super_admin)) && (id != user.id)
-    end
+  def admin_of?(user, permission)
+    has_correct_permission = highest_priority_role.get_permission(permission) && id != user.id
+
+    return has_correct_permission unless Rails.configuration.loadbalanced_configuration
+    return id != user.id if has_role? :super_admin
+    has_correct_permission && provider == user.provider && !user.has_role?(:super_admin)
   end
 
   def self.digest(string)

spec/controllers/admins_controller_spec.rb

@@ -239,6 +239,46 @@ describe AdminsController, type: :controller do
         expect(response).to redirect_to(admins_path)
       end
     end
+
+    context "POST permissions" do
+      it "allows a user with the correct permission to manage users" do
+        Role.create_new_role("test", "greenlight").update_all_role_permissions(can_manage_users: true)
+
+        @user2 = create(:user)
+        @user2.add_role(:test)
+
+        # Random manage user action test
+
+        @request.session[:user_id] = @user2.id
+
+        expect(@user.has_role?(:denied)).to eq(false)
+
+        post :ban_user, params: { user_uid: @user.uid }
+
+        @user.reload
+
+        expect(@user.has_role?(:denied)).to eq(true)
+        expect(flash[:success]).to be_present
+        expect(response).to redirect_to(admins_path)
+      end
+
+      it "doesn't allow a user with the incorrect permission to manage users" do
+        Role.create_new_role("test", "greenlight").update_all_role_permissions(can_manage_users: false)
+
+        @user2 = create(:user)
+        @user2.add_role(:test)
+
+        # Random manage user action test
+
+        @request.session[:user_id] = @user2.id
+
+        expect(@user.has_role?(:denied)).to eq(false)
+
+        post :ban_user, params: { user_uid: @user.uid }
+
+        expect(response).to render_template "errors/greenlight_error"
+      end
+    end
   end
 
   describe "User Design" do
@@ -446,6 +486,41 @@ describe AdminsController, type: :controller do
         expect(Rails.logger.level).to eq(2)
       end
     end
+
+    context "POST permissions" do
+      it "allows a user with the correct permission to edit site settings" do
+        Role.create_new_role("test", "greenlight").update_all_role_permissions(can_edit_site_settings: true)
+
+        @user2 = create(:user)
+        @user2.add_role(:test)
+
+        # Random edit site settings action test
+
+        @request.session[:user_id] = @user2.id
+
+        post :update_settings, params: { setting: "Shared Access", value: "false" }
+
+        feature = Setting.find_by(provider: "provider1").features.find_by(name: "Shared Access")
+
+        expect(feature[:value]).to eq("false")
+        expect(response).to redirect_to(admin_site_settings_path)
+      end
+
+      it "doesn't allow a user with the incorrect permission to edit site settings" do
+        Role.create_new_role("test", "greenlight").update_all_role_permissions(can_manage_users: true)
+
+        @user2 = create(:user)
+        @user2.add_role(:test)
+
+        # Random edit site settings action test
+
+        @request.session[:user_id] = @user2.id
+
+        post :update_settings, params: { setting: "Shared Access", value: "false" }
+
+        expect(response).to render_template "errors/greenlight_error"
+      end
+    end
   end
 
   describe "Roles" do
@@ -662,5 +737,47 @@ describe AdminsController, type: :controller do
         expect(response).to redirect_to admin_roles_path
       end
     end
+
+    context "POST permissions" do
+      it "allows a user with the correct permission to edit roles" do
+        Role.create_new_role("test", "greenlight").update_all_role_permissions(can_edit_roles: true)
+
+        @user2 = create(:user)
+        @user2.add_role(:test)
+
+        # Random edit roles action test
+
+        new_role = Role.create(name: "test2", priority: 2, provider: "provider1")
+        new_role.update_permission("can_edit_roles", "true")
+
+        @request.session[:user_id] = @user2.id
+
+        patch :update_role, params: { role_id: new_role.id, role: { name: "test3", can_edit_roles: false,
+          colour: "#45434", can_manage_users: true } }
+
+        new_role.reload
+        expect(new_role.name).to eq("test3")
+        expect(response).to redirect_to admin_roles_path(selected_role: new_role.id)
+      end
+
+      it "doesn't allow a user with the incorrect permission to edit roles" do
+        Role.create_new_role("test", "greenlight").update_all_role_permissions(can_manage_users: false)
+
+        @user2 = create(:user)
+        @user2.add_role(:test)
+
+        # Random edit roles action test
+
+        new_role = Role.create(name: "test2", priority: 2, provider: "provider1")
+        new_role.update_permission("can_edit_roles", "true")
+
+        @request.session[:user_id] = @user2.id
+
+        patch :update_role, params: { role_id: new_role.id, role: { name: "test3", can_edit_roles: false,
+          colour: "#45434", can_manage_users: true } }
+
+        expect(response).to render_template "errors/greenlight_error"
+      end
+    end
   end
 end

spec/models/user_spec.rb

@@ -167,18 +167,18 @@ describe User, type: :model do
       @admin = create(:user, provider: @user.provider)
       @admin.add_role :admin
 
-      expect(@admin.admin_of?(@user)).to be true
+      expect(@admin.admin_of?(@user, "can_manage_users")).to be true
 
       @super_admin = create(:user, provider: "test")
       @super_admin.add_role :super_admin
 
-      expect(@super_admin.admin_of?(@user)).to be true
+      expect(@super_admin.admin_of?(@user, "can_manage_users")).to be true
     end
 
     it "returns false if the user is NOT an admin of another" do
       @admin = create(:user)
 
-      expect(@admin.admin_of?(@user)).to be false
+      expect(@admin.admin_of?(@user, "can_manage_users")).to be false
     end
 
     it "should get the highest priority role" do