diff --git a/config/locales/services/en.yml b/config/locales/services/en.yml
index a6bd57a39..1075daf99 100644
--- a/config/locales/services/en.yml
+++ b/config/locales/services/en.yml
@@ -110,5 +110,4 @@ en:
users:
remove_role:
remove_self: Admins cannot remove roles from themselves.
- remove_super: Super Admin roles cannot be removed.
error: There was an issue removing this role. %{e_message}
diff --git a/config/locales/services/fr.yml b/config/locales/services/fr.yml
index 87312440c..ed402510e 100644
--- a/config/locales/services/fr.yml
+++ b/config/locales/services/fr.yml
@@ -108,5 +108,4 @@ fr:
users:
remove_role:
remove_self: Les administrateurs ne peuvent pas se retirer des rôles.
- remove_super: Les rôles de Super Admin ne peuvent pas être supprimés.
error: Un problème est survenu lors de la suppression de ce rôle. %{e_message}
diff --git a/config/locales/views/admin/en.yml b/config/locales/views/admin/en.yml
index 0f07cb324..b403e7cad 100644
--- a/config/locales/views/admin/en.yml
+++ b/config/locales/views/admin/en.yml
@@ -265,6 +265,7 @@ en:
locked: You can't remove this role.
remove: "Remove role:"
remove_confirm: Are you sure?
+ remove_confirm_super_admin: You are removing super admin access from this account. Do this carefully and always ensure you retain access to one or more super admin accounts
name:
admin: Admin
codeland_admin: Codeland Admin
diff --git a/config/locales/views/admin/fr.yml b/config/locales/views/admin/fr.yml
index 24fbf5319..2e518b115 100644
--- a/config/locales/views/admin/fr.yml
+++ b/config/locales/views/admin/fr.yml
@@ -265,6 +265,7 @@ fr:
locked: You can't remove this role.
remove: "Remove role:"
remove_confirm: Are you sure?
+ remove_confirm_super_admin: You are removing super admin access from this account. Do this carefully and always ensure you retain access to one or more super admin accounts
name:
admin: Admin
codeland_admin: Codeland Admin
diff --git a/cypress/e2e/seededFlows/adminFlows/users/manageRoles.spec.js b/cypress/e2e/seededFlows/adminFlows/users/manageRoles.spec.js
index 9068e837d..4983372b7 100644
--- a/cypress/e2e/seededFlows/adminFlows/users/manageRoles.spec.js
+++ b/cypress/e2e/seededFlows/adminFlows/users/manageRoles.spec.js
@@ -97,29 +97,6 @@ describe('Manage User Roles', () => {
);
checkUserStatus('Good standing');
});
-
- it('should not remove the Super Admin role', () => {
- checkUserStatus('Trusted');
-
- openRolesModal().within(() => {
- cy.findByRole('combobox', { name: 'Role' }).select('Super Admin');
- cy.findByRole('textbox', { name: 'Add a note to this action:' }).type(
- 'some reason',
- );
- cy.findByRole('button', { name: 'Add' }).click();
- });
-
- cy.findByRole('button', {
- name: `Super Admin You can't remove this role.`,
- })
- .as('superAdminButton')
- .click()
- .within(() => {
- cy.findByText(`You can't remove this role.`).should('exist');
- });
-
- cy.get('@superAdminButton').should('exist');
- });
});
describe('Adding Roles', () => {
diff --git a/spec/factories/roles.rb b/spec/factories/roles.rb
new file mode 100644
index 000000000..637705415
--- /dev/null
+++ b/spec/factories/roles.rb
@@ -0,0 +1,5 @@
+FactoryBot.define do
+ factory :role do
+ name { "admin" }
+ end
+end
diff --git a/spec/models/role_spec.rb b/spec/models/role_spec.rb
index f3dc0a51d..ad98ad64d 100644
--- a/spec/models/role_spec.rb
+++ b/spec/models/role_spec.rb
@@ -15,4 +15,25 @@ RSpec.describe Role do
expect(described_class::ROLES).to match_array(expected_roles)
end
end
+
+ describe "#super_admin?" do
+ let(:role) { create(:role, name: role_name) }
+ let(:role_name) { "super_admin" }
+
+ it "has a visible method" do
+ expect(role.respond_to?(:super_admin?)).to be true
+ end
+
+ it "returns true" do
+ expect(role.super_admin?).to be true
+ end
+
+ context "with different role" do
+ let(:role_name) { "admin" }
+
+ it "returns false" do
+ expect(role.super_admin?).to be false
+ end
+ end
+ end
end
diff --git a/spec/policies/role_policy_spec.rb b/spec/policies/role_policy_spec.rb
new file mode 100644
index 000000000..6c625315d
--- /dev/null
+++ b/spec/policies/role_policy_spec.rb
@@ -0,0 +1,25 @@
+require "rails_helper"
+
+describe RolePolicy do
+ subject(:role) { described_class }
+
+ let(:role_suspended) { build(:role, name: "suspended") }
+ let(:role_trusted) { build(:role, name: "trusted") }
+ let(:role_super_admin) { build(:role, name: "super_admin") }
+ let(:super_admin_user) { build(:user, :super_admin) }
+ let(:admin_user) { build(:user, :admin) }
+
+ permissions :remove_role? do
+ it "denies access if current role is suspended" do
+ expect(role).not_to permit(super_admin_user, role_suspended)
+ end
+
+ it "grants access if user is super admin" do
+ expect(role).to permit(super_admin_user, role_trusted)
+ end
+
+ it "denies access if user is admin and role is super_admin" do
+ expect(role).not_to permit(admin_user, role_super_admin)
+ end
+ end
+end
diff --git a/spec/requests/admin/users_manage_spec.rb b/spec/requests/admin/users_manage_spec.rb
index 1dccb5da7..232b68db8 100644
--- a/spec/requests/admin/users_manage_spec.rb
+++ b/spec/requests/admin/users_manage_spec.rb
@@ -1,6 +1,7 @@
require "rails_helper"
RSpec.describe "Admin::Users" do
+ # rubocop:disable RSpec/IndexedLet
let!(:user) { create(:user, twitter_username: nil, old_username: "username") }
let!(:user2) { create(:user, twitter_username: "Twitter") }
let(:user3) { create(:user) }
@@ -174,10 +175,10 @@ RSpec.describe "Admin::Users" do
end
it "removes non-admin roles from non-super_admin users", :aggregate_failures do
- user.add_role(:trusted)
+ role = user.add_role(:trusted)
expect do
- delete admin_user_path(user.id), params: { user_id: user.id, role: :trusted }
+ delete admin_user_path(user.id), params: { user_id: user.id, role_id: role.id }
end.to change(user.roles, :count).by(-1)
expect(user.has_trusted_role?).to be false
@@ -185,12 +186,12 @@ RSpec.describe "Admin::Users" do
end
it "removes the correct resource_admin_role from non-super_admin users", :aggregate_failures do
- user.add_role(:single_resource_admin, Comment)
+ role = user.add_role(:single_resource_admin, Comment)
user.add_role(:single_resource_admin, Broadcast)
expect do
delete admin_user_path(user.id),
- params: { user_id: user.id, role: :single_resource_admin, resource_type: Comment }
+ params: { user_id: user.id, role_id: role.id, resource_type: Comment }
end.to change(user.roles, :count).by(-1)
expect(user.single_resource_admin_for?(Comment)).to be false
@@ -198,22 +199,11 @@ RSpec.describe "Admin::Users" do
expect(request.flash["success"]).to include("successfully removed from the user!")
end
- it "does not allow super_admin roles to be removed", :aggregate_failures do
- user.add_role(:super_admin)
-
- expect do
- delete admin_user_path(user.id), params: { user_id: user.id, role: :super_admin }
- end.not_to change(user.roles, :count)
-
- expect(user.super_admin?).to be true
- expect(request.flash["danger"]).to include("cannot be removed.")
- end
-
it "does not allow a admins to remove a role from themselves", :aggregate_failures do
- super_admin.add_role(:trusted)
+ role = super_admin.add_role(:trusted)
expect do
- delete admin_user_path(super_admin.id), params: { user_id: super_admin.id, role: :trusted }
+ delete admin_user_path(super_admin.id), params: { user_id: super_admin.id, role_id: role.id }
end.not_to change(super_admin.roles, :count)
expect(super_admin.trusted?).to be true
@@ -308,4 +298,5 @@ RSpec.describe "Admin::Users" do
expect(super_admin.reload.unspent_credits_count).to eq 5
end
end
+ # rubocop:enable RSpec/IndexedLet
end
diff --git a/spec/services/users/remove_role_spec.rb b/spec/services/users/remove_role_spec.rb
index 71a95206e..3a383cbbe 100644
--- a/spec/services/users/remove_role_spec.rb
+++ b/spec/services/users/remove_role_spec.rb
@@ -3,19 +3,6 @@ require "rails_helper"
RSpec.describe Users::RemoveRole, type: :service do
let(:current_user) { create(:user, :admin) }
- context "when user is a super_admin" do
- it "does not remove super_admin roles and raises an error", :aggregate_failures do
- super_admin = create(:user, :super_admin)
- role = super_admin.roles.first.name.to_sym
- resource_type = nil
- args = { user: super_admin, role: role, resource_type: resource_type, admin: current_user }
- role_removal = described_class.call(**args)
-
- expect(role_removal.success).to be false
- expect(role_removal.error_message).to eq "Super Admin roles cannot be removed."
- end
- end
-
context "when current_user" do
it "does not remove roles and raises an error", :aggregate_failures do
role = current_user.roles.first