From 467947f1b52fbbbc8a98f58849539b5a15d162ad Mon Sep 17 00:00:00 2001 From: Ahmad Farhat Date: Wed, 6 May 2020 15:23:28 -0400 Subject: [PATCH] GRN2-xx: Switch the relation between users and roles to make queries cleaner and faster (#1299) * First steps * Fixes in account creation flow * Fixed most testcases * more test fixes * Fixed more test cases * Passing tests and rubocop * Added rake task to remove rooms --- app/assets/javascripts/user_edit.js | 66 +++----------- app/controllers/admins_controller.rb | 10 +-- app/controllers/application_controller.rb | 2 +- app/controllers/concerns/emailer.rb | 11 ++- app/controllers/concerns/populator.rb | 36 +++----- app/controllers/concerns/rolify.rb | 63 +++----------- app/controllers/recordings_controller.rb | 4 +- app/controllers/rooms_controller.rb | 2 +- app/controllers/sessions_controller.rb | 4 +- app/controllers/users_controller.rb | 10 ++- app/helpers/admins_helper.rb | 2 +- app/helpers/users_helper.rb | 9 +- app/models/ability.rb | 2 +- app/models/concerns/auth_values.rb | 2 +- app/models/role.rb | 6 +- app/models/user.rb | 87 ++++++------------- .../admins/components/_menu_buttons.html.erb | 2 +- app/views/admins/components/_roles.html.erb | 2 +- app/views/admins/components/_users.html.erb | 24 +---- app/views/shared/_header.html.erb | 4 +- app/views/users/components/_account.html.erb | 37 ++++---- config/locales/en.yml | 2 +- config/routes.rb | 2 +- .../20190314152108_rolify_create_roles.rb | 2 +- .../20200413150518_add_role_id_to_users.rb | 29 +++++++ db/schema.rb | 4 +- lib/tasks/room.rake | 27 ++++++ lib/tasks/user.rake | 4 +- .../account_activations_controller_spec.rb | 3 +- spec/controllers/admins_controller_spec.rb | 28 +++--- .../application_controller_spec.rb | 4 +- spec/controllers/rooms_controller_spec.rb | 30 +++---- spec/controllers/sessions_controller_spec.rb | 10 +-- spec/controllers/users_controller_spec.rb | 86 +++++------------- spec/factories.rb | 1 + spec/models/user_spec.rb | 45 +++------- spec/spec_helper.rb | 2 + 37 files changed, 262 insertions(+), 402 deletions(-) create mode 100644 db/migrate/20200413150518_add_role_id_to_users.rb create mode 100644 lib/tasks/room.rake diff --git a/app/assets/javascripts/user_edit.js b/app/assets/javascripts/user_edit.js index e1dee271..eca04cb2 100644 --- a/app/assets/javascripts/user_edit.js +++ b/app/assets/javascripts/user_edit.js @@ -18,61 +18,19 @@ $(document).on('turbolinks:load', function(){ var controller = $("body").data('controller'); var action = $("body").data('action'); if ((controller == "admins" && action == "edit_user") || (controller == "users" && action == "edit")) { - // Clear the role when the user clicks the x - $(".clear-role").click(clearRole) + // Hack to make it play nice with turbolinks + if ($("#role-dropdown:visible").length == 0){ + $(window).trigger('load.bs.select.data-api') + } - // When the user selects an item in the dropdown add the role to the user - $("#role-select-dropdown").change(function(data){ - var dropdown = $("#role-select-dropdown"); - var select_role_id = dropdown.val(); + // Check to see if the role dropdown was set up + if ($("#role-dropdown").length != 0){ + $("#role-dropdown").selectpicker('val', $("#user_role_id").val()) + } - if(select_role_id){ - // Disable the role in the dropdown - var selected_role = dropdown.find('[value=\"' + select_role_id + '\"]'); - selected_role.prop("disabled", true) - - // Add the role tag - var tag_container = $("#role-tag-container"); - tag_container.append("" + - selected_role.text() + ""); - - // Update the role ids input that gets submited on user update - var role_ids = $("#user_role_ids").val() - role_ids += " " + select_role_id - $("#user_role_ids").val(role_ids) - - // Add the clear role function to the tag - $("#user-role-tag_" + select_role_id).click(clearRole); - - // Reset the dropdown - dropdown.val(null) - } + // Update hidden field with new value + $("#role-dropdown").on("changed.bs.select", function(){ + $("#user_role_id").val($("#role-dropdown").selectpicker('val')) }) } -}) - -// This function removes the specfied role from a user -function clearRole(data){ - // Get the role id - var role_id = $(data.target).data("role-id"); - var role_tag = $("#user-role-tag_" + role_id); - - // Remove the role tag - $(role_tag).remove() - - // Update the role ids input - var role_ids = $("#user_role_ids").val() - var parsed_ids = role_ids.split(' ') - - var index = parsed_ids.indexOf(role_id.toString()); - - if (index > -1) { - parsed_ids.splice(index, 1); - } - - $("#user_role_ids").val(parsed_ids.join(' ')) - - // Enable the role in the role select dropdown - var selected_role = $("#role-select-dropdown").find('[value=\"' + role_id + '\"]'); - selected_role.prop("disabled", false) -} \ No newline at end of file +}) \ No newline at end of file diff --git a/app/controllers/admins_controller.rb b/app/controllers/admins_controller.rb index be8e8090..329ccf2b 100644 --- a/app/controllers/admins_controller.rb +++ b/app/controllers/admins_controller.rb @@ -86,23 +86,21 @@ class AdminsController < ApplicationController # POST /admins/ban/:user_uid def ban_user - @user.roles = [] - @user.add_role :denied + @user.set_role :denied redirect_back fallback_location: admins_path, flash: { success: I18n.t("administrator.flash.banned") } end # POST /admins/unban/:user_uid def unban_user - @user.remove_role :denied - @user.add_role :user + @user.set_role :user redirect_back fallback_location: admins_path, flash: { success: I18n.t("administrator.flash.unbanned") } end # POST /admins/approve/:user_uid def approve - @user.remove_role :pending + @user.set_role :user send_user_approved_email(@user) @@ -298,7 +296,7 @@ class AdminsController < ApplicationController flash[:alert] = I18n.t("administrator.roles.role_has_users", user_count: role.users.count) return redirect_to admin_roles_path(selected_role: role.id) elsif Role::RESERVED_ROLE_NAMES.include?(role) || role.provider != @user_domain || - role.priority <= current_user.highest_priority_role.priority + role.priority <= current_user.role.priority return redirect_to admin_roles_path(selected_role: role.id) else role.role_permissions.delete_all diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1e101f29..a0ddefba 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -26,7 +26,7 @@ class ApplicationController < ActionController::Base # Retrieves the current user. def current_user - @current_user ||= User.includes(:roles, :main_room).find_by(id: session[:user_id]) + @current_user ||= User.includes(:role, :main_room).find_by(id: session[:user_id]) if Rails.configuration.loadbalanced_configuration if @current_user && !@current_user.has_role?(:super_admin) && diff --git a/app/controllers/concerns/emailer.rb b/app/controllers/concerns/emailer.rb index 59540023..aeb48b1d 100644 --- a/app/controllers/concerns/emailer.rb +++ b/app/controllers/concerns/emailer.rb @@ -99,7 +99,6 @@ module Emailer def send_approval_user_signup_email(user) begin return unless Rails.configuration.enable_email_verification - admin_emails = admin_emails() UserMailer.approval_user_signup(user, admins_url(tab: "pending"), admin_emails, @settings).deliver_now unless admin_emails.empty? @@ -129,12 +128,12 @@ module Emailer end def admin_emails - admins = User.all_users_with_roles.where(roles: { role_permissions: { name: "can_manage_users", value: "true" } }) + roles = Role.where(provider: @user_domain, role_permissions: { name: "can_manage_users", value: "true" }) + .pluck(:name) - if Rails.configuration.loadbalanced_configuration - admins = admins.without_role(:super_admin) - .where(provider: @user_domain) - end + admins = User.with_role(roles - ["super_admin"]) + + admins = admins.where(provider: @user_domain) if Rails.configuration.loadbalanced_configuration admins.collect(&:email).join(",") end diff --git a/app/controllers/concerns/populator.rb b/app/controllers/concerns/populator.rb index 771fa25e..33fb9c3e 100644 --- a/app/controllers/concerns/populator.rb +++ b/app/controllers/concerns/populator.rb @@ -25,29 +25,22 @@ module Populator initial_user = case @tab when "active" - User.includes(:roles).without_role(:pending).without_role(:denied) + User.without_role([:pending, :denied]) when "deleted" - User.includes(:roles).deleted + User.deleted else - User.includes(:roles) + User.all end current_role = Role.find_by(name: @tab, provider: @user_domain) if @tab == "pending" || @tab == "denied" - initial_list = if current_user.has_role? :super_admin - initial_user.where.not(id: current_user.id) - else - initial_user.without_role(:super_admin).where.not(id: current_user.id) - end + initial_list = initial_user.without_role(:super_admin) unless current_user.has_role? :super_admin - if Rails.configuration.loadbalanced_configuration - initial_list.where(provider: @user_domain) - .admins_search(@search, current_role) - .admins_order(@order_column, @order_direction) - else - initial_list.admins_search(@search, current_role) - .admins_order(@order_column, @order_direction) - end + initial_list = initial_list.where(provider: @user_domain) if Rails.configuration.loadbalanced_configuration + + initial_list.where.not(id: current_user.id) + .admins_search(@search, current_role) + .admins_order(@order_column, @order_direction) end # Returns a list of rooms that are in the same context of the current user @@ -74,13 +67,12 @@ module Populator def shared_user_list roles_can_appear = [] Role.where(provider: @user_domain).each do |role| - roles_can_appear << role.name if role.get_permission("can_appear_in_share_list") && role.priority >= 0 + if role.get_permission("can_appear_in_share_list") && role.get_permission("can_create_rooms") && role.priority >= 0 + roles_can_appear << role.name + end end - initial_list = User.where.not(uid: current_user.uid) - .without_role(:pending) - .without_role(:denied) - .with_highest_priority_role(roles_can_appear) + initial_list = User.where.not(uid: current_user.uid).with_role(roles_can_appear) return initial_list unless Rails.configuration.loadbalanced_configuration initial_list.where(provider: @user_domain) @@ -88,7 +80,7 @@ module Populator # Returns a list of users that can merged into another user def merge_user_list - initial_list = User.where.not(uid: current_user.uid).without_role(:super_admin) + initial_list = User.without_role(:super_admin).where.not(uid: current_user.uid) return initial_list unless Rails.configuration.loadbalanced_configuration initial_list.where(provider: @user_domain) diff --git a/app/controllers/concerns/rolify.rb b/app/controllers/concerns/rolify.rb index 0dfff428..2074d4e9 100644 --- a/app/controllers/concerns/rolify.rb +++ b/app/controllers/concerns/rolify.rb @@ -46,60 +46,23 @@ module Rolify end # Updates a user's roles - def update_roles(roles) - # Check that the user can manage users - return true unless current_user.highest_priority_role.get_permission("can_manage_users") + def update_roles(role_id) + return true if role_id.blank? + # Check to make sure user can edit roles + return false unless current_user.role.get_permission("can_manage_users") - new_roles = roles.split(' ').map(&:to_i) - old_roles = @user.roles.pluck(:id).uniq + return true if @user.role_id == role_id - added_role_ids = new_roles - old_roles - removed_role_ids = old_roles - new_roles + new_role = Role.find_by(id: role_id, provider: @user_domain) + # Return false if new role doesn't exist + return false if new_role.nil? - added_roles = [] - removed_roles = [] - current_user_role = current_user.highest_priority_role - - # Check that the user has the permissions to add all the new roles - added_role_ids.each do |id| - role = Role.find(id) - - # Admins are able to add the admin role to other users. All other roles may only - # add roles with a higher priority - if (role.priority > current_user_role.priority || current_user_role.name == "admin") && - role.provider == @user_domain - added_roles << role - else - return false - end - end - - # Check that the user has the permissions to remove all the deleted roles - removed_role_ids.each do |id| - role = Role.find(id) - - # Admins are able to remove the admin role from other users. All other roles may only - # remove roles with a higher priority - if (role.priority > current_user_role.priority || current_user_role.name == "admin") && - role.provider == @user_domain - removed_roles << role - else - return false - end - end + return false if new_role.priority < current_user.role.priority # Send promoted/demoted emails - added_roles.each { |role| send_user_promoted_email(@user, role) if role.get_permission("send_promoted_email") } - removed_roles.each { |role| send_user_demoted_email(@user, role) if role.get_permission("send_demoted_email") } + send_user_promoted_email(@user, new_role) if new_role.get_permission("send_promoted_email") - # Update the roles - @user.roles.delete(removed_roles) - @user.roles << added_roles - - # Make sure each user always has at least the user role - @user.roles = [Role.find_by(name: "user", provider: @user_domain)] if @user.roles.count.zero? - - @user.save! + @user.update_attribute(:role_id, role_id) end # Updates a roles priority @@ -107,7 +70,7 @@ module Rolify user_role = Role.find_by(name: "user", provider: @user_domain) admin_role = Role.find_by(name: "admin", provider: @user_domain) - current_user_role = current_user.highest_priority_role + current_user_role = current_user.role # Users aren't allowed to update the priority of the admin or user roles return false if role_to_update.include?(user_role.id.to_s) || role_to_update.include?(admin_role.id.to_s) @@ -149,7 +112,7 @@ module Rolify # Update Permissions def update_permissions(role) - current_user_role = current_user.highest_priority_role + current_user_role = current_user.role # Checks that it is valid for the provider to update the role return false if role.priority <= current_user_role.priority || role.provider != @user_domain diff --git a/app/controllers/recordings_controller.rb b/app/controllers/recordings_controller.rb index fc82470e..93912b46 100644 --- a/app/controllers/recordings_controller.rb +++ b/app/controllers/recordings_controller.rb @@ -57,8 +57,6 @@ class RecordingsController < ApplicationController # Ensure the user is logged into the room they are accessing. def verify_room_ownership - if !@room.owned_by?(current_user) && !current_user&.highest_priority_role&.get_permission("can_manage_rooms_recordings") - redirect_to root_path - end + redirect_to root_path if !@room.owned_by?(current_user) && !current_user&.role&.get_permission("can_manage_rooms_recordings") end end diff --git a/app/controllers/rooms_controller.rb b/app/controllers/rooms_controller.rb index 7d2d048b..73e6597e 100644 --- a/app/controllers/rooms_controller.rb +++ b/app/controllers/rooms_controller.rb @@ -69,7 +69,7 @@ class RoomsController < ApplicationController # If its the current user's room if current_user && (@room.owned_by?(current_user) || @shared_room) - if current_user.highest_priority_role.get_permission("can_create_rooms") + if current_user.role.get_permission("can_create_rooms") # User is allowed to have rooms @search, @order_column, @order_direction, recs = recordings(@room.bbb_id, params.permit(:search, :column, :direction), true) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index d16b520b..ac7b34e0 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -218,7 +218,7 @@ class SessionsController < ApplicationController # Add pending role if approval method and is a new user if approval_registration && !@user_exists - user.add_role :pending + user.set_role :pending # Inform admins that a user signed up if emails are turned on send_approval_user_signup_email(user) @@ -228,6 +228,8 @@ class SessionsController < ApplicationController send_invite_user_signup_email(user) if invite_registration && !@user_exists + user.set_role :user unless @user_exists + login(user) if @auth['provider'] == "twitter" diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 9ce870bb..5311a517 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -47,7 +47,7 @@ class UsersController < ApplicationController # Set user to pending and redirect if Approval Registration is set if approval_registration - @user.add_role :pending + @user.set_role :pending return redirect_to root_path, flash: { success: I18n.t("registration.approval.signup") } unless Rails.configuration.enable_email_verification @@ -56,7 +56,11 @@ class UsersController < ApplicationController send_registration_email # Sign in automatically if email verification is disabled or if user is already verified. - login(@user) && return if !Rails.configuration.enable_email_verification || @user.email_verified + if !Rails.configuration.enable_email_verification || @user.email_verified + @user.set_role :user + + login(@user) && return + end send_activation_email(@user, @user.create_activation_token) @@ -116,7 +120,7 @@ class UsersController < ApplicationController user_locale(@user) - if update_roles(params[:user][:role_ids]) + if update_roles(params[:user][:role_id]) return redirect_to redirect_path, flash: { success: I18n.t("info_update_success") } else flash[:alert] = I18n.t("administrator.roles.invalid_assignment") diff --git a/app/helpers/admins_helper.rb b/app/helpers/admins_helper.rb index 6976a66b..b224c59f 100644 --- a/app/helpers/admins_helper.rb +++ b/app/helpers/admins_helper.rb @@ -110,6 +110,6 @@ module AdminsHelper # Roles def edit_disabled - @edit_disabled ||= @selected_role.priority <= current_user.highest_priority_role.priority + @edit_disabled ||= @selected_role.priority <= current_user.role.priority end end diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index c92ce09b..6e49a9a9 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -26,7 +26,7 @@ module UsersHelper end def disabled_roles(user) - current_user_role = current_user.highest_priority_role + current_user_role = current_user.role # Admins are able to remove the admin role from other admins # For all other roles they can only add/remove roles with a higher priority @@ -38,7 +38,7 @@ module UsersHelper .pluck(:id) end - user.roles.by_priority.pluck(:id) | disallowed_roles + [user.role.id] + disallowed_roles end # Returns language selection options for user edit @@ -52,6 +52,11 @@ module UsersHelper language_opts.sort end + # Returns a list of roles that the user can have + def role_options + Role.editable_roles(@user_domain).where("priority >= ?", current_user.role.priority) + end + # Parses markdown for rendering. def markdown(text) markdown = Redcarpet::Markdown.new(Redcarpet::Render::HTML, diff --git a/app/models/ability.rb b/app/models/ability.rb index 559d69f5..be439105 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -25,7 +25,7 @@ class Ability elsif user.has_role? :super_admin can :manage, :all else - highest_role = user.highest_priority_role + highest_role = user.role if highest_role.get_permission("can_edit_site_settings") can [:site_settings, :room_configuration, :update_settings, :update_room_configuration, :coloring, :registration_method], :admin diff --git a/app/models/concerns/auth_values.rb b/app/models/concerns/auth_values.rb index 0201959c..a3f719db 100644 --- a/app/models/concerns/auth_values.rb +++ b/app/models/concerns/auth_values.rb @@ -63,7 +63,7 @@ module AuthValues role_provider = auth['provider'] == "bn_launcher" ? auth['info']['customer'] : "greenlight" roles.each do |role_name| role = Role.find_by(provider: role_provider, name: role_name) - user.roles << role if !role.nil? && !user.has_role?(role_name) + user.role = role if !role.nil? && !user.has_role?(role_name) end end end diff --git a/app/models/role.rb b/app/models/role.rb index 124bcd8e..41c54bcf 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -17,10 +17,12 @@ # with BigBlueButton; if not, see . class Role < ApplicationRecord - has_and_belongs_to_many :users, join_table: :users_roles + has_and_belongs_to_many :users, join_table: :users_roles # Obsolete -- not used anymore has_many :role_permissions - default_scope { includes(:role_permissions).order(:priority) } + has_many :users + + default_scope { includes(:role_permissions).distinct.order(:priority) } scope :by_priority, -> { order(:priority) } scope :editable_roles, ->(provider) { where(provider: provider).where.not(name: %w[super_admin denied pending]) } diff --git a/app/models/user.rb b/app/models/user.rb index bdc25fa0..fd02bc97 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -31,7 +31,9 @@ class User < ApplicationRecord has_many :shared_access belongs_to :main_room, class_name: 'Room', foreign_key: :room_id, required: false - has_and_belongs_to_many :roles, join_table: :users_roles + has_and_belongs_to_many :roles, join_table: :users_roles # obsolete + + belongs_to :role, required: false validates :name, length: { maximum: 256 }, presence: true validates :provider, presence: true @@ -92,14 +94,12 @@ class User < ApplicationRecord end search_param = "%#{string}%" - joins("LEFT OUTER JOIN users_roles ON users_roles.user_id = users.id LEFT OUTER JOIN roles " \ - "ON roles.id = users_roles.role_id").distinct - .where(search_query, search: search_param, roles_search: role_search_param) + where(search_query, search: search_param, roles_search: role_search_param) end def self.admins_order(column, direction) # Arel.sql to avoid sql injection - order(Arel.sql("#{column} #{direction}")) + order(Arel.sql("users.#{column} #{direction}")) end # Returns a list of rooms ordered by last session (with nil rooms last) @@ -109,6 +109,7 @@ class User < ApplicationRecord # Activates an account and initialize a users main room def activate + set_role :user if role_id.nil? update_attributes(email_verified: true, activated_at: Time.zone.now, activation_digest: nil) end @@ -162,7 +163,7 @@ class User < ApplicationRecord end def admin_of?(user, permission) - has_correct_permission = highest_priority_role.get_permission(permission) && id != user.id + has_correct_permission = 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 @@ -170,70 +171,31 @@ class User < ApplicationRecord end # role functions - def highest_priority_role - roles.min_by(&:priority) - end + def set_role(role) # rubocop:disable Naming/AccessorMethodName + return if has_role?(role) - def add_role(role) - unless has_role?(role) - role_provider = Rails.configuration.loadbalanced_configuration ? provider : "greenlight" + new_role = Role.find_by(name: role, provider: role_provider) - new_role = Role.find_by(name: role, provider: role_provider) + return if new_role.nil? - if new_role.nil? - return if Role.duplicate_name(role, role_provider) || role.strip.empty? + create_home_room if main_room.nil? && new_role.get_permission("can_create_rooms") - new_role = Role.create_new_role(role, role_provider) - end + update_attribute(:role, new_role) - roles << new_role - - save! - end - end - - def remove_role(role) - if has_role?(role) - role_provider = Rails.configuration.loadbalanced_configuration ? provider : "greenlight" - - roles.delete(Role.find_by(name: role, provider: role_provider)) - save! - end + new_role end # This rule is disabled as the function name must be has_role? - # rubocop:disable Naming/PredicateName - def has_role?(role) - # rubocop:enable Naming/PredicateName - roles.each do |single_role| - return true if single_role.name.eql? role.to_s - end - - false + def has_role?(role_name) # rubocop:disable Naming/PredicateName + role&.name == role_name.to_s end def self.with_role(role) - User.all_users_with_roles.where(roles: { name: role }) + User.includes(:role).where(roles: { name: role }) end def self.without_role(role) - User.where.not(id: with_role(role).pluck(:id)) - end - - def self.with_highest_priority_role(role) - User.all_users_highest_priority_role.where(roles: { name: role }) - end - - def self.all_users_with_roles - User.joins("INNER JOIN users_roles ON users_roles.user_id = users.id INNER JOIN roles " \ - "ON roles.id = users_roles.role_id INNER JOIN role_permissions ON roles.id = role_permissions.role_id").distinct - end - - def self.all_users_highest_priority_role - User.joins("INNER JOIN (SELECT user_id, min(roles.priority) as role_priority FROM users_roles " \ - "INNER JOIN roles ON users_roles.role_id = roles.id GROUP BY user_id) as a ON " \ - "a.user_id = users.id INNER JOIN roles ON roles.priority = a.role_priority " \ - " INNER JOIN role_permissions ON roles.id = role_permissions.role_id").distinct + User.includes(:role).where.not(roles: { name: role }) end private @@ -246,15 +208,13 @@ class User < ApplicationRecord def setup_user # Initializes a room for the user and assign a BigBlueButton user id. id = "gl-#{(0...12).map { rand(65..90).chr }.join.downcase}" - room = Room.create!(owner: self, name: I18n.t("home_room")) - update_attributes(uid: id, main_room: room) + update_attributes(uid: id) # Initialize the user to use the default user role role_provider = Rails.configuration.loadbalanced_configuration ? provider : "greenlight" Role.create_default_roles(role_provider) if Role.where(provider: role_provider).count.zero? - add_role(:user) if roles.blank? end def check_if_email_can_be_blank @@ -266,4 +226,13 @@ class User < ApplicationRecord end end end + + def create_home_room + room = Room.create!(owner: self, name: I18n.t("home_room")) + update_attributes(main_room: room) + end + + def role_provider + Rails.configuration.loadbalanced_configuration ? provider : "greenlight" + end end diff --git a/app/views/admins/components/_menu_buttons.html.erb b/app/views/admins/components/_menu_buttons.html.erb index af8f3f12..858a1bf7 100644 --- a/app/views/admins/components/_menu_buttons.html.erb +++ b/app/views/admins/components/_menu_buttons.html.erb @@ -14,7 +14,7 @@ %>
- <% highest_role = current_user.highest_priority_role %> + <% highest_role = current_user.role %> <% highest_role.name %> <% if highest_role.get_permission("can_manage_users") || highest_role.name == "super_admin" %> <%= link_to admins_path, class: "list-group-item list-group-item-action dropdown-item #{"active" if active_page == "index"}" do %> diff --git a/app/views/admins/components/_roles.html.erb b/app/views/admins/components/_roles.html.erb index 77a316e1..f8ac2b2f 100644 --- a/app/views/admins/components/_roles.html.erb +++ b/app/views/admins/components/_roles.html.erb @@ -15,7 +15,7 @@
- <% current_role = current_user.highest_priority_role%> + <% current_role = current_user.role%>
diff --git a/app/views/admins/components/_users.html.erb b/app/views/admins/components/_users.html.erb index 8d46812e..582f3efd 100644 --- a/app/views/admins/components/_users.html.erb +++ b/app/views/admins/components/_users.html.erb @@ -13,21 +13,6 @@ # with BigBlueButton; if not, see . %> -<% -# BigBlueButton open source conferencing system - http://www.bigbluebutton.org/. -# Copyright (c) 2018 BigBlueButton Inc. and by respective authors (see below). -# This program is free software; you can redistribute it and/or modify it under the -# terms of the GNU Lesser General Public License as published by the Free Software -# Foundation; either version 3.0 of the License, or (at your option) any later -# version. -# -# BigBlueButton is distributed in the hope that it will be useful, but WITHOUT ANY -# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A -# PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details. -# You should have received a copy of the GNU Lesser General Public License along -# with BigBlueButton; if not, see . -%> - <% if @role.nil? %> <%= render "admins/components/manage_users_tags" %> <% else %> @@ -89,11 +74,10 @@ <%= user.email && user.email != "" ? user.email : user.username%> <%= user.provider %> - <% roles = user.roles().pluck(:name) %> - <%= render "admins/components/admins_role", role: user.highest_priority_role %> + <%= render "admins/components/admins_role", role: user.role %> - <% if !roles.include?("super_admin") %> + <% if !user.has_role?("super_admin") %>
@@ -106,14 +90,14 @@ - <% elsif roles.include?("denied") %> + <% elsif user.has_role?("denied") %> <%= button_to admin_unban_path(user_uid: user.uid), class: "dropdown-item", "data-disable": "" do %> <%= t("administrator.users.settings.unban") %> <% end %> - <% elsif roles.include?("pending") %> + <% elsif user.has_role?("pending") %> <%= button_to admin_approve_path(user_uid: user.uid), class: "dropdown-item", "data-disable": "" do %> <%= t("administrator.users.settings.approve") %> <% end %> diff --git a/app/views/shared/_header.html.erb b/app/views/shared/_header.html.erb index 963e1aa5..511251cb 100755 --- a/app/views/shared/_header.html.erb +++ b/app/views/shared/_header.html.erb @@ -38,7 +38,7 @@ <%= t("header.dropdown.home") %> <% end %> - <% if current_user.highest_priority_role.get_permission("can_create_rooms") %> + <% if current_user.role.get_permission("can_create_rooms") %> <% all_rec_page = params[:controller] == "users" && params[:action] == "recordings" ? "active" : "" %> <%= link_to get_user_recordings_path(current_user), class: "px-3 mx-1 mt-1 header-nav #{all_rec_page}" do %> <%= t("header.all_recordings") %> @@ -62,7 +62,7 @@ <%= link_to edit_user_path(current_user), class: "dropdown-item" do %> <%= t("header.dropdown.settings") %> <% end %> - <% highest_role = current_user.highest_priority_role %> + <% highest_role = current_user.role %> <% if highest_role.get_permission("can_manage_users") || highest_role.name == "super_admin" %> <%= link_to admins_path, class: "dropdown-item" do %> <%= t("header.dropdown.account_settings") %> diff --git a/app/views/users/components/_account.html.erb b/app/views/users/components/_account.html.erb index a058efd2..245979bc 100644 --- a/app/views/users/components/_account.html.erb +++ b/app/views/users/components/_account.html.erb @@ -13,7 +13,7 @@ # with BigBlueButton; if not, see . %> -<%= form_for @user, url: update_user_path, method: :patch do |f| %> +<%= form_for @user, url: update_user_path, method: :post do |f| %> <%= hidden_field_tag :setting, "account" %>
@@ -38,28 +38,21 @@ <%= f.label t("settings.account.language"), class: "form-label" %> <%= f.select :language, language_options, {}, { class: "form-control custom-select" } %> - <% current_user_role = current_user.highest_priority_role %> -
-
- <%= f.label t("settings.account.roles"), class: "form-label" %> - - <% if current_user_role.get_permission("can_manage_users") || current_user_role.name == "super_admin" %> - <% provider = Rails.configuration.loadbalanced_configuration ? current_user.provider : "greenlight" %> - <%= f.select :roles, Role.editable_roles(@user_domain).map{|role| [translated_role_name(role), role.id, {'data-colour' => role_colour(role)}]}.unshift(["", nil, {'data-colour' => nil}]), {disabled: disabled_roles(@user)}, { class: "form-control custom-select", id: "role-select-dropdown" } %> - <% end %> - <%= f.hidden_field :role_ids, id: "user_role_ids", value: @user.roles.by_priority.pluck(:id).uniq %> + <%= f.label t("settings.account.roles"), class: "form-label mt-5" %> + <% if current_user.role.get_permission("can_manage_users") %> + + + <%= f.hidden_field :role_id, id: "user_role_id", value: @user.role.id %> + <% else %> + " class="tag custom-role-tag"> + <%= translated_role_name(@user.role) %> + + <% end %> <%= f.label t("settings.account.image"), class: "form-label mt-5" %>
diff --git a/config/locales/en.yml b/config/locales/en.yml index 2cdc16ad..a80b1e0c 100755 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -534,7 +534,7 @@ en: provider: Provider image: Image image_url: Profile Image URL - roles: User Roles + roles: User Role subtitle: Update your Account Info title: Account Info delete: diff --git a/config/routes.rb b/config/routes.rb index dea1a367..8c9dd01e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -94,7 +94,7 @@ Rails.application.routes.draw do get '/:user_uid/edit', to: 'users#edit', as: :edit_user get '/:user_uid/change_password', to: 'users#change_password', as: :change_password get '/:user_uid/delete_account', to: 'users#delete_account', as: :delete_account - patch '/:user_uid/edit', to: 'users#update', as: :update_user + post '/:user_uid/edit', to: 'users#update', as: :update_user delete '/:user_uid', to: 'users#destroy', as: :delete_user # All user recordings diff --git a/db/migrate/20190314152108_rolify_create_roles.rb b/db/migrate/20190314152108_rolify_create_roles.rb index 437edf2c..dc0419d0 100644 --- a/db/migrate/20190314152108_rolify_create_roles.rb +++ b/db/migrate/20190314152108_rolify_create_roles.rb @@ -19,7 +19,7 @@ class RolifyCreateRoles < ActiveRecord::Migration[5.0] add_index(:users_roles, [:user_id, :role_id]) User.all.each do |user| - user.add_role(:user) if user.roles.blank? + user.set_role(:user) if user.roles.blank? end end end diff --git a/db/migrate/20200413150518_add_role_id_to_users.rb b/db/migrate/20200413150518_add_role_id_to_users.rb new file mode 100644 index 00000000..18b0863c --- /dev/null +++ b/db/migrate/20200413150518_add_role_id_to_users.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class MigrationProduct < ActiveRecord::Base + self.table_name = :users +end + +class SubMigrationProduct < ActiveRecord::Base + self.table_name = :roles +end + +class AddRoleIdToUsers < ActiveRecord::Migration[5.2] + def change + reversible do |dir| + dir.up do + add_reference :users, :role, index: true + + MigrationProduct.where(role_id: nil).each do |user| + highest_role = SubMigrationProduct.joins("INNER JOIN users_roles ON users_roles.role_id = roles.id") + .where("users_roles.user_id = '#{user.id}'").min_by(&:priority).id + user.update_attributes(role_id: highest_role) unless highest_role.nil? + end + end + + dir.down do + remove_reference :users, :role, index: true + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 9385d04c..89cc2aae 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_01_30_144841) do +ActiveRecord::Schema.define(version: 2020_04_13_150518) do create_table "features", force: :cascade do |t| t.integer "setting_id" @@ -120,11 +120,13 @@ ActiveRecord::Schema.define(version: 2020_01_30_144841) do t.string "activation_digest" t.datetime "activated_at" t.boolean "deleted", default: false, null: false + t.integer "role_id" t.index ["created_at"], name: "index_users_on_created_at" t.index ["deleted"], name: "index_users_on_deleted" t.index ["email"], name: "index_users_on_email" t.index ["password_digest"], name: "index_users_on_password_digest", unique: true t.index ["provider"], name: "index_users_on_provider" + t.index ["role_id"], name: "index_users_on_role_id" t.index ["room_id"], name: "index_users_on_room_id" end diff --git a/lib/tasks/room.rake b/lib/tasks/room.rake new file mode 100644 index 00000000..b922b83b --- /dev/null +++ b/lib/tasks/room.rake @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'bigbluebutton_api' + +namespace :room do + desc "Removes all rooms for users that can't create rooms" + task :remove, [:include_used] => :environment do |_task, args| + roles = Role.where(role_permissions: { name: "can_create_rooms", value: "false" }).pluck(:name) + users = User.with_role(roles) + users.each do |user| + puts "Destroying #{user.uid} rooms" + user.rooms.each do |room| + if room.sessions.positive? && args[:include_used] != "true" + puts "Skipping room #{room.uid}" + next + end + + begin + room.destroy(true) + puts "Destroying room #{room.uid}" + rescue => e + puts "Failed to remove room #{room.uid} - #{e}" + end + end + end + end +end diff --git a/lib/tasks/user.rake b/lib/tasks/user.rake index dfe85c6b..bfba6f06 100644 --- a/lib/tasks/user.rake +++ b/lib/tasks/user.rake @@ -38,9 +38,9 @@ namespace :user do if u[:role] == "super_admin" user.remove_role(:user) - user.add_role(:super_admin) + user.set_role(:super_admin) elsif u[:role] == "admin" - user.add_role(:admin) + user.set_role(:admin) end puts "Account succesfully created." diff --git a/spec/controllers/account_activations_controller_spec.rb b/spec/controllers/account_activations_controller_spec.rb index 6fd27e12..9a78abb0 100644 --- a/spec/controllers/account_activations_controller_spec.rb +++ b/spec/controllers/account_activations_controller_spec.rb @@ -70,7 +70,8 @@ describe AccountActivationsController, type: :controller do it "redirects a pending user to root with a flash" do @user = create(:user, email_verified: false, provider: "greenlight") - @user.add_role :pending + @user.set_role :pending + @user.reload get :edit, params: { token: @user.create_activation_token } diff --git a/spec/controllers/admins_controller_spec.rb b/spec/controllers/admins_controller_spec.rb index c9e4e25c..3f5730dd 100644 --- a/spec/controllers/admins_controller_spec.rb +++ b/spec/controllers/admins_controller_spec.rb @@ -25,7 +25,7 @@ describe AdminsController, type: :controller do @user = create(:user, provider: "provider1") @admin = create(:user, provider: "provider1") - @admin.add_role :admin + @admin.set_role :admin end describe "User Roles" do @@ -78,7 +78,7 @@ describe AdminsController, type: :controller do context "POST #unban" do it "unbans the user from the application" do @request.session[:user_id] = @admin.id - @user.add_role :denied + @user.set_role :denied expect(@user.has_role?(:denied)).to eq(true) @@ -153,7 +153,7 @@ describe AdminsController, type: :controller do it "approves a pending user" do @request.session[:user_id] = @admin.id - @user.add_role :pending + @user.set_role :pending post :approve, params: { user_uid: @user.uid } @@ -167,7 +167,7 @@ describe AdminsController, type: :controller do it "sends the user an email telling them theyre approved" do @request.session[:user_id] = @admin.id - @user.add_role :pending + @user.set_role :pending params = { user_uid: @user.uid } expect { post :approve, params: params }.to change { ActionMailer::Base.deliveries.count }.by(1) end @@ -245,7 +245,7 @@ describe AdminsController, type: :controller do Role.create_new_role("test", "greenlight").update_all_role_permissions(can_manage_users: true) @user2 = create(:user) - @user2.add_role(:test) + @user2.set_role(:test) # Random manage user action test @@ -266,7 +266,7 @@ describe AdminsController, type: :controller do Role.create_new_role("test", "greenlight").update_all_role_permissions(can_manage_users: false) @user2 = create(:user) - @user2.add_role(:test) + @user2.set_role(:test) # Random manage user action test @@ -450,7 +450,7 @@ describe AdminsController, type: :controller do @request.session[:user_id] = @admin.id - @admin.add_role :super_admin + @admin.set_role :super_admin @admin.update_attribute(:provider, "greenlight") @user2 = create(:user, provider: "provider1") @user3 = create(:user, provider: "provider1") @@ -479,7 +479,7 @@ describe AdminsController, type: :controller do it "changes the log level" do @request.session[:user_id] = @admin.id - @admin.add_role :super_admin + @admin.set_role :super_admin expect(Rails.logger.level).to eq(0) post :log_level, params: { value: 2 } @@ -492,7 +492,7 @@ describe AdminsController, type: :controller do Role.create_new_role("test", "greenlight").update_all_role_permissions(can_edit_site_settings: true) @user2 = create(:user) - @user2.add_role(:test) + @user2.set_role(:test) # Random edit site settings action test @@ -510,7 +510,7 @@ describe AdminsController, type: :controller do Role.create_new_role("test", "greenlight").update_all_role_permissions(can_manage_users: true) @user2 = create(:user) - @user2.add_role(:test) + @user2.set_role(:test) # Random edit site settings action test @@ -610,7 +610,7 @@ describe AdminsController, type: :controller do new_role2 = Role.create_new_role("test2", "provider1") new_role2.update_permission("can_edit_roles", "true") - @user.roles << new_role2 + @user.role = new_role2 @user.save! @request.session[:user_id] = @user.id @@ -657,7 +657,7 @@ describe AdminsController, type: :controller do new_role2 = Role.create(name: "test2", priority: 2, provider: "provider1") new_role2.update_permission("can_edit_roles", "true") - @user.roles << new_role2 + @user.role = new_role2 @user.save! @request.session[:user_id] = @user.id @@ -743,7 +743,7 @@ describe AdminsController, type: :controller do Role.create_new_role("test", "greenlight").update_all_role_permissions(can_edit_roles: true) @user2 = create(:user) - @user2.add_role(:test) + @user2.set_role(:test) # Random edit roles action test @@ -764,7 +764,7 @@ describe AdminsController, type: :controller do Role.create_new_role("test", "greenlight").update_all_role_permissions(can_manage_users: false) @user2 = create(:user) - @user2.add_role(:test) + @user2.set_role(:test) # Random edit roles action test diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 7b4da958..3b3705f9 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -43,7 +43,7 @@ describe ApplicationController do end it "redirects a banned user to a 401 and logs them out" do - @user.add_role :denied + @user.set_role :denied @request.session[:user_id] = @user.id get :index @@ -53,7 +53,7 @@ describe ApplicationController do end it "redirects a pending user to a 401 and logs them out" do - @user.add_role :pending + @user.set_role :pending @request.session[:user_id] = @user.id get :index diff --git a/spec/controllers/rooms_controller_spec.rb b/spec/controllers/rooms_controller_spec.rb index cc4f825d..4fdaafe7 100644 --- a/spec/controllers/rooms_controller_spec.rb +++ b/spec/controllers/rooms_controller_spec.rb @@ -64,7 +64,7 @@ describe RoomsController, type: :controller do end it "should render cant_create_rooms if user doesn't have permission to create rooms" do - user_role = @user.highest_priority_role + user_role = @user.role user_role.update_permission("can_create_rooms", "false") user_role.save! @@ -117,7 +117,7 @@ describe RoomsController, type: :controller do it "redirects to admin if user is a super_admin" do @request.session[:user_id] = @owner.id - @owner.add_role :super_admin + @owner.set_role :super_admin get :show, params: { room_uid: @owner.main_room, search: :none } @@ -140,7 +140,7 @@ describe RoomsController, type: :controller do it "redirects to root if owner is pending" do @request.session[:user_id] = @owner.id - @owner.add_role :pending + @owner.set_role :pending get :show, params: { room_uid: @owner.main_room, search: :none } @@ -149,7 +149,7 @@ describe RoomsController, type: :controller do it "redirects to root if owner is banned" do @request.session[:user_id] = @owner.id - @owner.add_role :denied + @owner.set_role :denied get :show, params: { room_uid: @owner.main_room, search: :none } @@ -406,7 +406,7 @@ describe RoomsController, type: :controller do it "redirects to root if owner is pending" do @request.session[:user_id] = @owner.id - @owner.add_role :pending + @owner.set_role :pending post :join, params: { room_uid: @room } @@ -415,7 +415,7 @@ describe RoomsController, type: :controller do it "redirects to root if owner is banned" do @request.session[:user_id] = @owner.id - @owner.add_role :denied + @owner.set_role :denied post :join, params: { room_uid: @room } @@ -456,7 +456,7 @@ describe RoomsController, type: :controller do it "allows admin to delete room" do @admin = create(:user) - @admin.add_role :admin + @admin.set_role :admin @request.session[:user_id] = @admin.id expect do @@ -468,7 +468,7 @@ describe RoomsController, type: :controller do it "does not allow admin to delete a users home room" do @admin = create(:user) - @admin.add_role :admin + @admin.set_role :admin @request.session[:user_id] = @admin.id expect do @@ -483,7 +483,7 @@ describe RoomsController, type: :controller do allow_any_instance_of(User).to receive(:admin_of?).and_return(false) @admin = create(:user) - @admin.add_role :admin + @admin.set_role :admin @request.session[:user_id] = @admin.id expect do @@ -527,7 +527,7 @@ describe RoomsController, type: :controller do it "redirects to join path if admin" do @admin = create(:user) - @admin.add_role :admin + @admin.set_role :admin @request.session[:user_id] = @admin.id post :start, params: { room_uid: @user.main_room } @@ -538,7 +538,7 @@ describe RoomsController, type: :controller do it "redirects to root path if not admin of current user" do allow_any_instance_of(User).to receive(:admin_of?).and_return(false) @admin = create(:user) - @admin.add_role :admin + @admin.set_role :admin @request.session[:user_id] = @admin.id post :start, params: { room_uid: @user.main_room } @@ -587,7 +587,7 @@ describe RoomsController, type: :controller do it "allows admin to update room settings" do @admin = create(:user) - @admin.add_role :admin + @admin.set_role :admin @request.session[:user_id] = @admin.id room_params = { "mute_on_join": "1", "name": @secondary_room.name } @@ -603,7 +603,7 @@ describe RoomsController, type: :controller do it "does not allow admins from a different context to update room settings" do allow_any_instance_of(User).to receive(:admin_of?).and_return(false) @admin = create(:user) - @admin.add_role :admin + @admin.set_role :admin @request.session[:user_id] = @admin.id room_params = { "mute_on_join": "1", "name": @secondary_room.name } @@ -743,7 +743,7 @@ describe RoomsController, type: :controller do it "allows admins to update room access" do @admin = create(:user) - @admin.add_role :admin + @admin.set_role :admin @request.session[:user_id] = @admin.id post :shared_access, params: { room_uid: @room.uid, add: [@user1.uid] } @@ -756,7 +756,7 @@ describe RoomsController, type: :controller do it "redirects to root path if not admin of current user" do allow_any_instance_of(User).to receive(:admin_of?).and_return(false) @admin = create(:user) - @admin.add_role :admin + @admin.set_role :admin @request.session[:user_id] = @admin.id post :shared_access, params: { room_uid: @room.uid, add: [] } diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 4e93c185..9c0aabc2 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -221,7 +221,7 @@ describe SessionsController, type: :controller do it "redirects to the admins page for admins" do user = create(:user, provider: "greenlight", password: "example", password_confirmation: 'example') - user.add_role :super_admin + user.set_role :super_admin post :create, params: { session: { @@ -235,7 +235,7 @@ describe SessionsController, type: :controller do end it "should migrate old rooms from the twitter account to the new user" do - twitter_user = User.create(name: "Twitter User", email: "user@twitter.com", image: "example.png", + twitter_user = create(:user, name: "Twitter User", email: "user@twitter.com", image: "example.png", username: "twitteruser", email_verified: true, provider: 'twitter', social_uid: "twitter-user") room = Room.new(name: "Test") @@ -383,7 +383,7 @@ describe SessionsController, type: :controller do it "should notify twitter users that twitter is deprecated" do allow(Rails.configuration).to receive(:allow_user_signup).and_return(true) - twitter_user = User.create(name: "Twitter User", email: "user@twitter.com", image: "example.png", + twitter_user = create(:user, name: "Twitter User", email: "user@twitter.com", image: "example.png", username: "twitteruser", email_verified: true, provider: 'twitter', social_uid: "twitter-user") request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:twitter] @@ -394,7 +394,7 @@ describe SessionsController, type: :controller do end it "should migrate rooms from the twitter account to the google account" do - twitter_user = User.create(name: "Twitter User", email: "user@twitter.com", image: "example.png", + twitter_user = create(:user, name: "Twitter User", email: "user@twitter.com", image: "example.png", username: "twitteruser", email_verified: true, provider: 'twitter', social_uid: "twitter-user") room = Room.new(name: "Test") @@ -419,7 +419,7 @@ describe SessionsController, type: :controller do allow(Rails.configuration).to receive(:enable_email_verification).and_return(true) @user = create(:user, provider: "greenlight") @admin = create(:user, provider: "greenlight", email: "test@example.com") - @admin.add_role :admin + @admin.set_role :admin end it "should notify admin on new user signup with approve/reject registration" do diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index ee078f41..7a685af2 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -75,7 +75,7 @@ describe UsersController, type: :controller do controller.instance_variable_set(:@user_domain, "provider1") user = create(:user, provider: "provider1") - user.add_role :admin + user.set_role :admin user2 = create(:user, provider: "provider1") @request.session[:user_id] = user.id @@ -174,7 +174,7 @@ describe UsersController, type: :controller do allow(Rails.configuration).to receive(:allow_user_signup).and_return(true) @user = create(:user, provider: "greenlight") @admin = create(:user, provider: "greenlight", email: "test@example.com") - @admin.add_role :admin + @admin.set_role :admin end it "should notify admins that user signed up" do @@ -232,7 +232,7 @@ describe UsersController, type: :controller do allow(Rails.configuration).to receive(:allow_user_signup).and_return(true) @user = create(:user, provider: "greenlight") @admin = create(:user, provider: "greenlight", email: "test@example.com") - @admin.add_role :admin + @admin.set_role :admin end it "allows any user to sign up" do @@ -278,13 +278,13 @@ describe UsersController, type: :controller do end end - describe "PATCH #update" do + describe "POST #update" do it "properly updates user attributes" do user = create(:user) @request.session[:user_id] = user.id params = random_valid_user_params - patch :update, params: params.merge!(user_uid: user) + post :update, params: params.merge!(user_uid: user) user.reload expect(user.name).to eql(params[:user][:name]) @@ -297,7 +297,7 @@ describe UsersController, type: :controller do @user = create(:user) @request.session[:user_id] = @user.id - patch :update, params: invalid_params.merge!(user_uid: @user) + post :update, params: invalid_params.merge!(user_uid: @user) expect(response).to render_template(:edit) end @@ -306,7 +306,7 @@ describe UsersController, type: :controller do user = create(:user) @request.session[:user_id] = user.id - user_role = user.highest_priority_role + user_role = user.role user_role.update_permission("can_manage_users", "true") @@ -315,30 +315,7 @@ describe UsersController, type: :controller do tmp_role = Role.create(name: "test", priority: -4, provider: "greenlight") params = random_valid_user_params - patch :update, params: params.merge!(user_uid: user, user: { role_ids: tmp_role.id.to_s }) - - expect(flash[:alert]).to eq(I18n.t("administrator.roles.invalid_assignment")) - expect(response).to render_template(:edit) - end - - it "should fail to update roles if a user tries to remove a role with a higher priority than their own" do - user = create(:user) - admin = create(:user) - - admin.add_role :admin - - @request.session[:user_id] = user.id - - user_role = user.highest_priority_role - - user_role.update_permission("can_manage_users", "true") - - user_role.save! - - params = random_valid_user_params - patch :update, params: params.merge!(user_uid: admin, user: { role_ids: "" }) - - user.reload + post :update, params: params.merge!(user_uid: user, user: { role_id: tmp_role.id.to_s }) expect(flash[:alert]).to eq(I18n.t("administrator.roles.invalid_assignment")) expect(response).to render_template(:edit) @@ -350,53 +327,30 @@ describe UsersController, type: :controller do user = create(:user) admin = create(:user) - admin.add_role :admin + admin.set_role :admin @request.session[:user_id] = admin.id tmp_role1 = Role.create(name: "test1", priority: 2, provider: "greenlight") tmp_role1.update_permission("send_promoted_email", "true") - tmp_role2 = Role.create(name: "test2", priority: 3, provider: "greenlight") params = random_valid_user_params - params = params.merge!(user_uid: user, user: { role_ids: "#{tmp_role1.id} #{tmp_role2.id}" }) + params = params.merge!(user_uid: user, user: { role_id: tmp_role1.id.to_s }) - expect { patch :update, params: params }.to change { ActionMailer::Base.deliveries.count }.by(1) + expect { post :update, params: params }.to change { ActionMailer::Base.deliveries.count }.by(1) user.reload - expect(user.roles.count).to eq(2) - expect(user.highest_priority_role.name).to eq("test1") - expect(response).to redirect_to(admins_path) - end - - it "all users must at least have the user role" do - allow(Rails.configuration).to receive(:enable_email_verification).and_return(true) - - user = create(:user) - admin = create(:user) - - admin.add_role :admin - - tmp_role1 = Role.create(name: "test1", priority: 2, provider: "greenlight") - tmp_role1.update_permission("send_demoted_email", "true") - user.roles << tmp_role1 - user.save! - - @request.session[:user_id] = admin.id - - params = random_valid_user_params - params = params.merge!(user_uid: user, user: { role_ids: "" }) - - expect { patch :update, params: params }.to change { ActionMailer::Base.deliveries.count }.by(1) - expect(user.roles.count).to eq(1) - expect(user.highest_priority_role.name).to eq("user") + expect(user.role.name).to eq("test1") expect(response).to redirect_to(admins_path) end end end describe "DELETE #user" do - before { allow(Rails.configuration).to receive(:allow_user_signup).and_return(true) } + before do + allow(Rails.configuration).to receive(:allow_user_signup).and_return(true) + Role.create_default_roles("provider1") + end it "permanently deletes user" do user = create(:user) @@ -416,7 +370,7 @@ describe UsersController, type: :controller do user = create(:user, provider: "provider1") admin = create(:user, provider: "provider1") - admin.add_role :admin + admin.set_role :admin @request.session[:user_id] = admin.id delete :destroy, params: { user_uid: user.uid } @@ -434,7 +388,7 @@ describe UsersController, type: :controller do user = create(:user, provider: "provider1") admin = create(:user, provider: "provider1") - admin.add_role :admin + admin.set_role :admin @request.session[:user_id] = admin.id delete :destroy, params: { user_uid: user.uid, permanent: "true" } @@ -452,7 +406,7 @@ describe UsersController, type: :controller do user = create(:user, provider: "provider1") admin = create(:user, provider: "provider1") - admin.add_role :admin + admin.set_role :admin @request.session[:user_id] = admin.id uid = user.main_room.uid @@ -473,7 +427,7 @@ describe UsersController, type: :controller do user = create(:user, provider: "provider1") admin = create(:user, provider: "provider2") - admin.add_role :admin + admin.set_role :admin @request.session[:user_id] = admin.id delete :destroy, params: { user_uid: user.uid } diff --git a/spec/factories.rb b/spec/factories.rb index 70515f6a..3998c120 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -29,6 +29,7 @@ FactoryBot.define do accepted_terms { true } email_verified { true } activated_at { Time.zone.now } + role { set_role(:user) } end factory :room do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 051e3052..9670b997 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -170,12 +170,12 @@ describe User, type: :model do allow_any_instance_of(User).to receive(:greenlight_account?).and_return(true) @admin = create(:user, provider: @user.provider) - @admin.add_role :admin + @admin.set_role :admin expect(@admin.admin_of?(@user, "can_manage_users")).to be true @super_admin = create(:user, provider: "test") - @super_admin.add_role :super_admin + @super_admin.set_role :super_admin expect(@super_admin.admin_of?(@user, "can_manage_users")).to be true end @@ -188,32 +188,16 @@ describe User, type: :model do it "should get the highest priority role" do @admin = create(:user, provider: @user.provider) - @admin.add_role :admin + @admin.set_role :admin - expect(@admin.highest_priority_role.name).to eq("admin") - end - - it "should skip adding the role if the user already has the role" do - @admin = create(:user, provider: @user.provider) - @admin.add_role :admin - @admin.add_role :admin - - expect(@admin.roles.count).to eq(2) + expect(@admin.role.name).to eq("admin") end it "should add the role if the user doesn't already have the role" do @admin = create(:user, provider: @user.provider) - @admin.add_role :admin + @admin.set_role :admin - expect(@admin.roles.count).to eq(2) - end - - it "should remove the role if the user has the role assigned to them" do - @admin = create(:user, provider: @user.provider) - @admin.add_role :admin - @admin.remove_role :admin - - expect(@admin.roles.count).to eq(1) + expect(@admin.has_role?(:admin)).to eq(true) end it "has_role? should return false if the user doesn't have the role" do @@ -222,7 +206,7 @@ describe User, type: :model do it "has_role? should return true if the user has the role" do @admin = create(:user, provider: @user.provider) - @admin.add_role :admin + @admin.set_role :admin expect(@admin.has_role?(:admin)).to eq(true) end @@ -230,8 +214,8 @@ describe User, type: :model do it "with_role should return all users with the role" do @admin1 = create(:user, provider: @user.provider) @admin2 = create(:user, provider: @user.provider) - @admin1.add_role :admin - @admin2.add_role :admin + @admin1.set_role :admin + @admin2.set_role :admin expect(User.with_role(:admin).count).to eq(2) end @@ -239,18 +223,11 @@ describe User, type: :model do it "without_role should return all users without the role" do @admin1 = create(:user, provider: @user.provider) @admin2 = create(:user, provider: @user.provider) - @admin1.add_role :admin - @admin2.add_role :admin + @admin1.set_role :admin + @admin2.set_role :admin expect(User.without_role(:admin).count).to eq(1) end - - it "all_users_with_roles should return all users with at least one role" do - @admin1 = create(:user, provider: @user.provider) - @admin2 = create(:user, provider: @user.provider) - - expect(User.all_users_with_roles.count).to eq(3) - end end context 'blank email' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0e59b070..c8c43a17 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -108,6 +108,8 @@ RSpec.configure do |config| ", headers: {}) if ENV['LOADBALANCER_ENDPOINT'] + + Role.create_default_roles("greenlight") end # rspec-expectations config goes here. You can use an alternate