diff --git a/app/assets/javascripts/app/models/user.coffee b/app/assets/javascripts/app/models/user.coffee index f1ed97298..423035030 100644 --- a/app/assets/javascripts/app/models/user.coffee +++ b/app/assets/javascripts/app/models/user.coffee @@ -344,9 +344,12 @@ class App.User extends App.Model @sameOrganization?(requester) isChangeableBy: (requester) -> + # full access for admins return true if requester.permission('admin.user') - # allow agents to change customers + # forbid non-agents to change users return false if !requester.permission('ticket.agent') + # allow agents to change customers only + return false if @permission(['admin.user', 'ticket.agent']) @permission('ticket.customer') isDeleteableBy: (requester) -> diff --git a/spec/system/manage/users_spec.rb b/spec/system/manage/users_spec.rb index 8072ba604..fe10293a9 100644 --- a/spec/system/manage/users_spec.rb +++ b/spec/system/manage/users_spec.rb @@ -110,4 +110,73 @@ RSpec.describe 'Manage > Users', type: :system do end end end + + describe 'check user edit permissions', authenticated_as: -> { user } do + + shared_examples 'user permission' do |allow| + it(allow ? 'allows editing' : 'forbids editing') do + visit "#user/profile/#{record.id}" + find('.js-action .icon-arrow-down').click + selector = '.js-action [data-type="edit"]' + expect(page).to(allow ? have_css(selector) : have_no_css(selector)) + end + end + + context 'when admin tries to change admin' do + let(:user) { create(:admin) } + let(:record) { create(:admin) } + + include_examples 'user permission', true + end + + context 'when admin tries to change agent' do + let(:user) { create(:admin) } + let(:record) { create(:agent) } + + include_examples 'user permission', true + end + + context 'when admin tries to change customer' do + let(:user) { create(:admin) } + let(:record) { create(:customer) } + + include_examples 'user permission', true + end + + context 'when agent tries to change admin' do + let(:user) { create(:agent) } + let(:record) { create(:admin) } + + include_examples 'user permission', false + end + + context 'when agent tries to change agent' do + let(:user) { create(:agent) } + let(:record) { create(:agent) } + + include_examples 'user permission', false + end + + context 'when agent tries to change customer' do + let(:user) { create(:agent) } + let(:record) { create(:customer) } + + include_examples 'user permission', true + end + + context 'when agent tries to change customer who is also admin' do + let(:user) { create(:agent) } + let(:record) { create(:customer, role_ids: Role.signup_role_ids.push(Role.find_by(name: 'Admin').id)) } + + include_examples 'user permission', false + end + + context 'when agent tries to change customer who is also agent' do + let(:user) { create(:agent) } + let(:record) { create(:customer, role_ids: Role.signup_role_ids.push(Role.find_by(name: 'Agent').id)) } + + include_examples 'user permission', false + end + + end end