From b097ee726d02109cc0412bf265950868905d79c3 Mon Sep 17 00:00:00 2001 From: Cesium Date: Wed, 28 Sep 2022 15:40:03 -0700 Subject: [PATCH 1/6] AO3 6141 disable user role checkboxes --- app/helpers/admin_helper.rb | 20 +++++++++++++++++++ .../admin/admin_users/_user_form.html.erb | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/helpers/admin_helper.rb b/app/helpers/admin_helper.rb index cab5fda40ce..dc4b6539591 100644 --- a/app/helpers/admin_helper.rb +++ b/app/helpers/admin_helper.rb @@ -64,6 +64,26 @@ def admin_can_update_user_roles? policy(User).permitted_attributes.include?(roles: []) end + def admin_can_edit_user_role(role) + return false unless logged_in_as_admin? + return true if current_admin.roles.include? "superadmin" + + if current_admin.roles.include? "tag_wrangling" + return role.name == "tag_wrangler" + end + + if current_admin.roles.include? "policy_and_abuse" + return role.name == "protected_user" + end + + if current_admin.roles.include? "open_doors" + return ["archivist", "opendoors"].include? role.name + end + + return false + + end + def admin_can_update_user_email? return unless logged_in_as_admin? diff --git a/app/views/admin/admin_users/_user_form.html.erb b/app/views/admin/admin_users/_user_form.html.erb index bebb0516add..6a3b50f3394 100644 --- a/app/views/admin/admin_users/_user_form.html.erb +++ b/app/views/admin/admin_users/_user_form.html.erb @@ -12,7 +12,7 @@ <%= text_field_tag "user[email]", user.email, title: ts("Email"), disabled: !admin_can_update_user_email? %> <% for role in @roles %> - <%= check_box_tag "user[roles][]", role.id, user.roles.include?(role), title: role.name, id: "user_roles_#{role.id}", disabled: !admin_can_update_user_roles? %> + <%= check_box_tag "user[roles][]", role.id, user.roles.include?(role), title: role.name, id: "user_roles_#{role.id}", disabled: !admin_can_edit_user_role(role) %> <% end %> From e80ca40f8a7d5b07e437752ec660dd73fe82f10c Mon Sep 17 00:00:00 2001 From: Cesium Date: Wed, 28 Sep 2022 15:50:06 -0700 Subject: [PATCH 2/6] please hound --- app/helpers/admin_helper.rb | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/app/helpers/admin_helper.rb b/app/helpers/admin_helper.rb index dc4b6539591..7e482a3a667 100644 --- a/app/helpers/admin_helper.rb +++ b/app/helpers/admin_helper.rb @@ -65,23 +65,13 @@ def admin_can_update_user_roles? end def admin_can_edit_user_role(role) - return false unless logged_in_as_admin? return true if current_admin.roles.include? "superadmin" - - if current_admin.roles.include? "tag_wrangling" - return role.name == "tag_wrangler" - end - - if current_admin.roles.include? "policy_and_abuse" - return role.name == "protected_user" - end - + return role.name == "tag_wrangler" if current_admin.roles.include? "tag_wrangling" + return role.name == "protected_user" if current_admin.roles.include? "policy_and_abuse" if current_admin.roles.include? "open_doors" - return ["archivist", "opendoors"].include? role.name + return %w[archivist opendoors].include? role.name end - return false - end def admin_can_update_user_email? From 257d5b9bde4af2a3bfab7d41c57e33c6fec5d83a Mon Sep 17 00:00:00 2001 From: Cesium Date: Tue, 6 Dec 2022 22:52:48 -0800 Subject: [PATCH 3/6] move logic to userpolicy class --- app/helpers/admin_helper.rb | 10 ---------- app/policies/user_policy.rb | 20 +++++++++++++++++++ .../admin/admin_users/_user_form.html.erb | 2 +- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/app/helpers/admin_helper.rb b/app/helpers/admin_helper.rb index 7e482a3a667..cab5fda40ce 100644 --- a/app/helpers/admin_helper.rb +++ b/app/helpers/admin_helper.rb @@ -64,16 +64,6 @@ def admin_can_update_user_roles? policy(User).permitted_attributes.include?(roles: []) end - def admin_can_edit_user_role(role) - return true if current_admin.roles.include? "superadmin" - return role.name == "tag_wrangler" if current_admin.roles.include? "tag_wrangling" - return role.name == "protected_user" if current_admin.roles.include? "policy_and_abuse" - if current_admin.roles.include? "open_doors" - return %w[archivist opendoors].include? role.name - end - return false - end - def admin_can_update_user_email? return unless logged_in_as_admin? diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 4515eb73888..5e90d261d51 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -33,6 +33,26 @@ def permitted_attributes ALLOWED_ATTRIBUTES_BY_ROLES.values_at(*user.roles).compact.flatten end + def can_edit_case_superadmin?(role) + user_has_roles?(%w[superadmin]) + end + + def can_edit_case_tag_wrangling?(role) + user_has_roles?(%w[tag_wrangling]) && role.name == "tag_wrangler" + end + + def can_edit_case_policy_and_abuse?(role) + user_has_roles?(%w[policy_and_abuse]) && role.name == "protected_user" + end + + def can_edit_case_open_doors?(role) + user_has_roles?(%w[open_doors]) && (role.name == "archivist" || role.name == "opendoors") + end + + def can_edit_user_role?(role) + can_edit_case_superadmin?(role) || can_edit_case_tag_wrangling?(role) || can_edit_case_policy_and_abuse?(role) || can_edit_case_open_doors?(role) + end + alias index? can_manage_users? alias bulk_search? can_manage_users? alias show? can_manage_users? diff --git a/app/views/admin/admin_users/_user_form.html.erb b/app/views/admin/admin_users/_user_form.html.erb index 6a3b50f3394..f0b9d6241ca 100644 --- a/app/views/admin/admin_users/_user_form.html.erb +++ b/app/views/admin/admin_users/_user_form.html.erb @@ -12,7 +12,7 @@ <%= text_field_tag "user[email]", user.email, title: ts("Email"), disabled: !admin_can_update_user_email? %> <% for role in @roles %> - <%= check_box_tag "user[roles][]", role.id, user.roles.include?(role), title: role.name, id: "user_roles_#{role.id}", disabled: !admin_can_edit_user_role(role) %> + <%= check_box_tag "user[roles][]", role.id, user.roles.include?(role), title: role.name, id: "user_roles_#{role.id}", disabled: !(policy(User).can_edit_user_role?(role)) %> <% end %> From 5d40fbc5cb925a9b58f298e7ed800feaae4e8b20 Mon Sep 17 00:00:00 2001 From: Cesium Date: Tue, 6 Dec 2022 22:54:49 -0800 Subject: [PATCH 4/6] please hound --- app/policies/user_policy.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 5e90d261d51..98043823b35 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -33,7 +33,7 @@ def permitted_attributes ALLOWED_ATTRIBUTES_BY_ROLES.values_at(*user.roles).compact.flatten end - def can_edit_case_superadmin?(role) + def can_edit_case_superadmin? user_has_roles?(%w[superadmin]) end @@ -50,7 +50,7 @@ def can_edit_case_open_doors?(role) end def can_edit_user_role?(role) - can_edit_case_superadmin?(role) || can_edit_case_tag_wrangling?(role) || can_edit_case_policy_and_abuse?(role) || can_edit_case_open_doors?(role) + can_edit_case_superadmin? || can_edit_case_tag_wrangling?(role) || can_edit_case_policy_and_abuse?(role) || can_edit_case_open_doors?(role) end alias index? can_manage_users? From 878f520ff5658e12e1274e080bec40fcbf63b1f0 Mon Sep 17 00:00:00 2001 From: Cesium Date: Tue, 13 Dec 2022 00:01:39 -0800 Subject: [PATCH 5/6] use hash for checking that admin role can edit user role --- app/policies/user_policy.rb | 26 +++++++------------ .../admin/admin_users/_user_form.html.erb | 2 +- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 98043823b35..d5d08453b77 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -21,6 +21,14 @@ class UserPolicy < ApplicationPolicy "tag_wrangling" => [roles: []] }.freeze + # Define which admin roles can edit which user roles. + ALLOWED_USER_ROLES_BY_ADMIN_ROLES = { + "open_doors" => %w[archivist opendoors], + "policy_and_abuse" => %w[no_resets protected_user], + "superadmin" => %w[archivist no_resets official opendoors protected_user tag_wrangler], + "tag_wrangling" => %w[tag_wrangler] + } + def can_manage_users? user_has_roles?(MANAGE_ROLES) end @@ -33,24 +41,8 @@ def permitted_attributes ALLOWED_ATTRIBUTES_BY_ROLES.values_at(*user.roles).compact.flatten end - def can_edit_case_superadmin? - user_has_roles?(%w[superadmin]) - end - - def can_edit_case_tag_wrangling?(role) - user_has_roles?(%w[tag_wrangling]) && role.name == "tag_wrangler" - end - - def can_edit_case_policy_and_abuse?(role) - user_has_roles?(%w[policy_and_abuse]) && role.name == "protected_user" - end - - def can_edit_case_open_doors?(role) - user_has_roles?(%w[open_doors]) && (role.name == "archivist" || role.name == "opendoors") - end - def can_edit_user_role?(role) - can_edit_case_superadmin? || can_edit_case_tag_wrangling?(role) || can_edit_case_policy_and_abuse?(role) || can_edit_case_open_doors?(role) + ALLOWED_USER_ROLES_BY_ADMIN_ROLES.values_at(*user.roles).compact.flatten.include?(role.name) end alias index? can_manage_users? diff --git a/app/views/admin/admin_users/_user_form.html.erb b/app/views/admin/admin_users/_user_form.html.erb index f0b9d6241ca..addc627d6f4 100644 --- a/app/views/admin/admin_users/_user_form.html.erb +++ b/app/views/admin/admin_users/_user_form.html.erb @@ -12,7 +12,7 @@ <%= text_field_tag "user[email]", user.email, title: ts("Email"), disabled: !admin_can_update_user_email? %> <% for role in @roles %> - <%= check_box_tag "user[roles][]", role.id, user.roles.include?(role), title: role.name, id: "user_roles_#{role.id}", disabled: !(policy(User).can_edit_user_role?(role)) %> + <%= check_box_tag "user[roles][]", role.id, user.roles.include?(role), title: role.name, id: "user_roles_#{role.id}", disabled: !policy(User).can_edit_user_role?(role) %> <% end %> From d4860b21fdf7873a9f3fb64cc140aa2bb470c7b4 Mon Sep 17 00:00:00 2001 From: Cesium Date: Tue, 13 Dec 2022 00:14:00 -0800 Subject: [PATCH 6/6] please hound --- app/policies/user_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index d5d08453b77..0975aaebaec 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -27,7 +27,7 @@ class UserPolicy < ApplicationPolicy "policy_and_abuse" => %w[no_resets protected_user], "superadmin" => %w[archivist no_resets official opendoors protected_user tag_wrangler], "tag_wrangling" => %w[tag_wrangler] - } + }.freeze def can_manage_users? user_has_roles?(MANAGE_ROLES)