diff --git a/app/assets/javascripts/room.js b/app/assets/javascripts/room.js index e3afbd8a..0d94b92d 100644 --- a/app/assets/javascripts/room.js +++ b/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 diff --git a/app/controllers/admins_controller.rb b/app/controllers/admins_controller.rb index ddef4f5f..52f8b7d3 100644 --- a/app/controllers/admins_controller.rb +++ b/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 diff --git a/app/controllers/concerns/joiner.rb b/app/controllers/concerns/joiner.rb index 1836851b..22643122 100644 --- a/app/controllers/concerns/joiner.rb +++ b/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"] diff --git a/app/controllers/rooms_controller.rb b/app/controllers/rooms_controller.rb index edb1f294..e1bc1fdb 100644 --- a/app/controllers/rooms_controller.rb +++ b/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 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 3779c55e..f463dff9 100644 --- a/app/controllers/users_controller.rb +++ b/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 diff --git a/app/models/ability.rb b/app/models/ability.rb index 541d6940..a4f98ab8 100644 --- a/app/models/ability.rb +++ b/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") diff --git a/app/models/user.rb b/app/models/user.rb index f12672e7..466e7820 100644 --- a/app/models/user.rb +++ b/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) diff --git a/spec/controllers/admins_controller_spec.rb b/spec/controllers/admins_controller_spec.rb index 36620710..c9e4e25c 100644 --- a/spec/controllers/admins_controller_spec.rb +++ b/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 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7ab823f5..8046565e 100644 --- a/spec/models/user_spec.rb +++ b/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