From 666231db6c5a30a54d0087ce0447047f1e7a3023 Mon Sep 17 00:00:00 2001
From: shawn-higgins1 <23224097+shawn-higgins1@users.noreply.github.com>
Date: Tue, 27 Aug 2019 11:30:25 -0400
Subject: [PATCH] Change permissions from columns to table entries (#762)
---
app/controllers/concerns/emailer.rb | 2 +-
app/controllers/concerns/rolify.rb | 19 +++++--
app/controllers/recordings_controller.rb | 2 +-
app/controllers/rooms_controller.rb | 2 +-
app/models/ability.rb | 9 ++--
app/models/role.rb | 51 ++++++++++++++++---
app/models/role_permission.rb | 5 ++
app/models/user.rb | 8 +--
.../admins/components/_menu_buttons.html.erb | 6 +--
app/views/admins/components/_roles.html.erb | 26 +++++-----
app/views/shared/_header.html.erb | 8 +--
app/views/users/components/_account.html.erb | 6 +--
.../20190822134205_create_role_permissions.rb | 31 +++++++++++
db/schema.rb | 18 ++++---
spec/controllers/admins_controller_spec.rb | 24 +++++----
spec/controllers/rooms_controller_spec.rb | 2 +-
spec/controllers/users_controller_spec.rb | 10 ++--
17 files changed, 163 insertions(+), 66 deletions(-)
create mode 100644 app/models/role_permission.rb
create mode 100644 db/migrate/20190822134205_create_role_permissions.rb
diff --git a/app/controllers/concerns/emailer.rb b/app/controllers/concerns/emailer.rb
index 378ad62e..39ee1ea3 100644
--- a/app/controllers/concerns/emailer.rb
+++ b/app/controllers/concerns/emailer.rb
@@ -128,7 +128,7 @@ module Emailer
end
def admin_emails
- admins = User.all_users_with_roles.where(roles: { can_manage_users: true })
+ admins = User.all_users_with_roles.where(roles: { role_permissions: { name: "can_manage_users", value: "true" } })
if Rails.configuration.loadbalanced_configuration
admins = admins.without_role(:super_admin)
diff --git a/app/controllers/concerns/rolify.rb b/app/controllers/concerns/rolify.rb
index 589a6d49..1fab4bc5 100644
--- a/app/controllers/concerns/rolify.rb
+++ b/app/controllers/concerns/rolify.rb
@@ -48,7 +48,7 @@ module Rolify
# Updates a user's roles
def update_roles(roles)
# Check that the user can manage users
- return true unless current_user.highest_priority_role.can_manage_users
+ return true unless current_user.highest_priority_role.get_permission("can_manage_users")
new_roles = roles.split(' ').map(&:to_i)
old_roles = @user.roles.pluck(:id)
@@ -89,8 +89,8 @@ module Rolify
end
# Send promoted/demoted emails
- added_roles.each { |role| send_user_promoted_email(@user, role) if role.send_promoted_email }
- removed_roles.each { |role| send_user_demoted_email(@user, role) if role.send_demoted_email }
+ 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") }
# Update the roles
@user.roles.delete(removed_roles)
@@ -143,6 +143,16 @@ module Rolify
permission_params = params.require(:role).permit(:can_create_rooms, :send_promoted_email,
:send_demoted_email, :can_edit_site_settings, :can_edit_roles, :can_manage_users, :colour)
+ permission_params.transform_values! do |v|
+ if v == "0"
+ "false"
+ elsif v == "1"
+ "true"
+ else
+ v
+ end
+ end
+
# Role is a default role so users can't change the name
role_params[:name] = role.name if Role::RESERVED_ROLE_NAMES.include?(role.name)
@@ -154,7 +164,8 @@ module Rolify
return false
end
- role.update(permission_params)
+ role.update(colour: permission_params[:colour])
+ role.update_all_role_permissions(permission_params)
role.save!
end
diff --git a/app/controllers/recordings_controller.rb b/app/controllers/recordings_controller.rb
index b002c918..d5d3c80f 100644
--- a/app/controllers/recordings_controller.rb
+++ b/app/controllers/recordings_controller.rb
@@ -58,7 +58,7 @@ class RecordingsController < ApplicationController
# Ensure the user is logged into the room they are accessing.
def verify_room_ownership
if !current_user || (!@room.owned_by?(current_user) &&
- !current_user.highest_priority_role.can_edit_site_settings &&
+ !current_user.highest_priority_role.get_permission("can_edit_site_settings") &&
!current_user.has_role?(:super_admin))
redirect_to root_path
end
diff --git a/app/controllers/rooms_controller.rb b/app/controllers/rooms_controller.rb
index 9d9b14dd..e9e1b0c6 100644
--- a/app/controllers/rooms_controller.rb
+++ b/app/controllers/rooms_controller.rb
@@ -63,7 +63,7 @@ class RoomsController < ApplicationController
# If its the current user's room
if current_user && @room.owned_by?(current_user)
- if current_user.highest_priority_role.can_create_rooms
+ if current_user.highest_priority_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/models/ability.rb b/app/models/ability.rb
index 2c9fb9a2..8be21b91 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -26,20 +26,21 @@ class Ability
can :manage, :all
else
highest_role = user.highest_priority_role
- if highest_role.can_edit_site_settings
+ if highest_role.get_permission("can_edit_site_settings")
can [:index, :site_settings, :server_recordings, :update_settings, :coloring, :registration_method], :admin
end
- if highest_role.can_edit_roles
+ if highest_role.get_permission("can_edit_roles")
can [:index, :roles, :new_role, :change_role_order, :update_role, :delete_role], :admin
end
- if highest_role.can_manage_users
+ if highest_role.get_permission("can_manage_users")
can [:index, :roles, :edit_user, :promote, :demote, :ban_user, :unban_user,
:approve, :invite, :reset], :admin
end
- if !highest_role.can_edit_site_settings && !highest_role.can_edit_roles && !highest_role.can_manage_users
+ if !highest_role.get_permission("can_edit_site_settings") && !highest_role.get_permission("can_edit_roles") &&
+ !highest_role.get_permission("can_manage_users")
cannot :manage, AdminsController
end
end
diff --git a/app/models/role.rb b/app/models/role.rb
index 6f0e932d..a4b1331f 100644
--- a/app/models/role.rb
+++ b/app/models/role.rb
@@ -18,6 +18,7 @@
class Role < ApplicationRecord
has_and_belongs_to_many :users, join_table: :users_roles
+ has_many :role_permissions
default_scope { order(:priority) }
scope :by_priority, -> { order(:priority) }
@@ -30,15 +31,18 @@ class Role < ApplicationRecord
end
def self.create_default_roles(provider)
- Role.create(name: "user", provider: provider, priority: 1, can_create_rooms: true, colour: "#868e96")
- Role.create(name: "admin", provider: provider, priority: 0, can_create_rooms: true, send_promoted_email: true,
+ Role.create(name: "user", provider: provider, priority: 1, colour: "#868e96")
+ .update_all_role_permissions(can_create_rooms: true)
+ Role.create(name: "admin", provider: provider, priority: 0, colour: "#f1c40f")
+ .update_all_role_permissions(can_create_rooms: true, send_promoted_email: true,
send_demoted_email: true, can_edit_site_settings: true,
- can_edit_roles: true, can_manage_users: true, colour: "#f1c40f")
- Role.create(name: "pending", provider: provider, priority: -1, colour: "#17a2b8")
- Role.create(name: "denied", provider: provider, priority: -1, colour: "#343a40")
- Role.create(name: "super_admin", provider: provider, priority: -2, can_create_rooms: true,
+ can_edit_roles: true, can_manage_users: true)
+ Role.create(name: "pending", provider: provider, priority: -1, colour: "#17a2b8").update_all_role_permissions
+ Role.create(name: "denied", provider: provider, priority: -1, colour: "#343a40").update_all_role_permissions
+ Role.create(name: "super_admin", provider: provider, priority: -2, colour: "#cd201f")
+ .update_all_role_permissions(can_create_rooms: true,
send_promoted_email: true, send_demoted_email: true, can_edit_site_settings: true,
- can_edit_roles: true, can_manage_users: true, colour: "#cd201f")
+ can_edit_roles: true, can_manage_users: true)
end
def self.create_new_role(role_name, provider)
@@ -56,4 +60,37 @@ class Role < ApplicationRecord
role
end
+
+ def update_all_role_permissions(permissions = {})
+ update_permission("can_create_rooms", permissions[:can_create_rooms].to_s)
+ update_permission("send_promoted_email", permissions[:send_promoted_email].to_s)
+ update_permission("send_demoted_email", permissions[:send_demoted_email].to_s)
+ update_permission("can_edit_site_settings", permissions[:can_edit_site_settings].to_s)
+ update_permission("can_edit_roles", permissions[:can_edit_roles].to_s)
+ update_permission("can_manage_users", permissions[:can_manage_users].to_s)
+ end
+
+ # Updates the value of the permission and enables it
+ def update_permission(name, value)
+ permission = role_permissions.find_or_create_by!(name: name)
+
+ permission.update_attributes(value: value, enabled: true)
+ end
+
+ # Returns the value if enabled or the default if not enabled
+ def get_permission(name, return_boolean = true)
+ permission = role_permissions.find_or_create_by!(name: name)
+
+ value = if permission[:enabled]
+ permission[:value]
+ else
+ "false"
+ end
+
+ if return_boolean
+ value == "true"
+ else
+ value
+ end
+ end
end
diff --git a/app/models/role_permission.rb b/app/models/role_permission.rb
new file mode 100644
index 00000000..058b900c
--- /dev/null
+++ b/app/models/role_permission.rb
@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class RolePermission < ApplicationRecord
+ belongs_to :role
+end
diff --git a/app/models/user.rb b/app/models/user.rb
index d24997d9..52e39bbe 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -29,7 +29,7 @@ class User < ApplicationRecord
has_many :rooms
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, -> { includes :role_permissions }, join_table: :users_roles
validates :name, length: { maximum: 256 }, presence: true
validates :provider, presence: true
@@ -163,11 +163,11 @@ class User < ApplicationRecord
if has_role? :super_admin
id != user.id
else
- highest_priority_role.can_manage_users && (id != user.id) && (provider == user.provider) &&
+ highest_priority_role.get_permission("can_manage_users") && (id != user.id) && (provider == user.provider) &&
(!user.has_role? :super_admin)
end
else
- (highest_priority_role.can_manage_users || (has_role? :super_admin)) && (id != user.id)
+ (highest_priority_role.get_permission("can_manage_users") || (has_role? :super_admin)) && (id != user.id)
end
end
@@ -230,7 +230,7 @@ class User < ApplicationRecord
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")
+ "ON roles.id = users_roles.role_id INNER JOIN role_permissions ON roles.id = role_permissions.role_id").distinct
end
private
diff --git a/app/views/admins/components/_menu_buttons.html.erb b/app/views/admins/components/_menu_buttons.html.erb
index 395cc3d4..17bbc460 100644
--- a/app/views/admins/components/_menu_buttons.html.erb
+++ b/app/views/admins/components/_menu_buttons.html.erb
@@ -16,12 +16,12 @@
<% highest_role = current_user.highest_priority_role %>
<% highest_role.name %>
- <% if highest_role.can_manage_users || highest_role.name == "super_admin" %>
+ <% 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 %>
<%= t("administrator.users.title") %>
<% end %>
<% end %>
- <% if highest_role.can_edit_site_settings || highest_role.name == "super_admin" %>
+ <% if highest_role.get_permission("can_edit_site_settings") || highest_role.name == "super_admin" %>
<%= link_to admin_recordings_path, class: "list-group-item list-group-item-action dropdown-item #{"active" if active_page == "server_recordings"}" do %>
<%= t("administrator.recordings.title") %>
<% end %>
@@ -29,7 +29,7 @@
<%= t("administrator.site_settings.title") %>
<% end %>
<% end %>
- <% if highest_role.can_edit_roles || highest_role.name == "super_admin" %>
+ <% if highest_role.get_permission("can_edit_roles") || highest_role.name == "super_admin" %>
<%= link_to admin_roles_path, class: "list-group-item list-group-item-action dropdown-item #{"active" if active_page == "roles"}" do %>
<%= t("administrator.roles.title") %>
<% end %>
diff --git a/app/views/admins/components/_roles.html.erb b/app/views/admins/components/_roles.html.erb
index 9d8cf77b..51f5cf14 100644
--- a/app/views/admins/components/_roles.html.erb
+++ b/app/views/admins/components/_roles.html.erb
@@ -33,7 +33,7 @@
">
- <%= form_for(@selected_role, url: admin_update_role_path(@selected_role.id), method: :post) do |f| %>
+ <%= form_with model: @selected_role, url: admin_update_role_path(@selected_role.id), method: :post do |f| %>
<%= f.label t('administrator.roles.name'), class: "form-label" %>
<%= f.text_field :name, class: 'form-control mb-3', value: translated_role_name(@selected_role), readonly: edit_disabled || @selected_role.name == "user" || @selected_role.name == "admin", required: true %>
@@ -48,34 +48,34 @@
-