Fixes #1758 disallow assigning admin/admin.*/ticket.agent if default_at_signup is set to true

app/controllers/application_controller/renders_models.rb

@@ -45,11 +45,12 @@ module ApplicationController::RendersModels
 
     generic_object.with_lock do
 
+      # set relations
+      generic_object.associations_from_param(params)
+
       # set attributes
       generic_object.update!(clean_params)
 
-      # set relations
-      generic_object.associations_from_param(params)
     end
 
     if response_expand?

app/models/role.rb

@@ -10,12 +10,12 @@ class Role < ApplicationModel
   include Role::Assets
 
   has_and_belongs_to_many :users, after_add: :cache_update, after_remove: :cache_update
-  has_and_belongs_to_many :permissions, after_add: :cache_update, after_remove: :cache_update, before_add: :validate_agent_limit_by_permission, before_remove: :last_admin_check_by_permission
+  has_and_belongs_to_many :permissions, after_add: :cache_update, after_remove: :cache_update, before_update: :cache_update, after_update: :cache_update, before_add: :validate_agent_limit_by_permission, before_remove: :last_admin_check_by_permission
   validates               :name,  presence: true
   store                   :preferences
 
-  before_create  :validate_permissions
-  before_update  :validate_permissions, :last_admin_check_by_attribute, :validate_agent_limit_by_attributes
+  before_create  :validate_permissions, :check_default_at_signup_permissions
+  before_update  :validate_permissions, :last_admin_check_by_attribute, :validate_agent_limit_by_attributes, :check_default_at_signup_permissions
 
   association_attributes_ignored :users
 
@@ -153,6 +153,7 @@ returns
   private
 
   def validate_permissions
+    Rails.logger.debug "self permission: #{self.permission_ids}"
     return true if !self.permission_ids
     permission_ids.each do |permission_id|
       permission = Permission.lookup(id: permission_id)
@@ -213,4 +214,13 @@ returns
     true
   end
 
+  def check_default_at_signup_permissions
+    all_permissions = Permission.all.pluck(:id)
+    admin_permissions = Permission.where('name LIKE ? OR name = ?', 'admin%', 'ticket.agent').pluck(:id) # admin.*/ticket.agent permissions
+    normal_permissions = (all_permissions - admin_permissions) | (admin_permissions - all_permissions) # all other permissions besides admin.*/ticket.agent
+    return true if default_at_signup != true # means if default_at_signup = false, no need further checks
+    return true if self.permission_ids.all? { |i| normal_permissions.include? i } # allow user to choose only normal permissions
+    raise Exceptions::UnprocessableEntity, 'Cannot set default at signup when role has admin and ticket agent properties'
+  end
+
 end

test/unit/role_test.rb

@@ -138,4 +138,87 @@ class RoleTest < ActiveSupport::TestCase
     assert(role.with_permission?(['test-with-permission2', 'some_other_permission']))
   end
 
+  test 'default_at_signup' do
+
+    agent_role = Role.find_by(name: 'Agent')
+    assert_raises(Exceptions::UnprocessableEntity) do
+      agent_role.default_at_signup = true
+      agent_role.save!
+    end
+
+    admin_role = Role.find_by(name: 'Admin')
+    assert_raises(Exceptions::UnprocessableEntity) do
+      admin_role.default_at_signup = true
+      admin_role.save!
+    end
+
+    assert_raises(Exceptions::UnprocessableEntity) do
+      Role.create!(
+        name: 'Test1',
+        note: 'Test1 Role.',
+        default_at_signup: true,
+        permissions: [Permission.find_by(name: 'admin')],
+        updated_by_id: 1,
+        created_by_id: 1
+      )
+    end
+
+    role = Role.create!(
+      name: 'Test1',
+      note: 'Test1 Role.',
+      default_at_signup: false,
+      permissions: [Permission.find_by(name: 'admin')],
+      updated_by_id: 1,
+      created_by_id: 1
+    )
+    assert(role)
+
+    permissions = Permission.where('name LIKE ? OR name = ?', 'admin%', 'ticket.agent').pluck(:name) # get all administrative permissions
+    permissions.each do |type|
+
+      assert_raises(Exceptions::UnprocessableEntity) do
+        Role.create!(
+          name: "Test1_#{type}",
+          note: 'Test1 Role.',
+          default_at_signup: true,
+          permissions: [Permission.find_by(name: type)],
+          updated_by_id: 1,
+          created_by_id: 1
+        )
+      end
+
+      role = Role.create!(
+        name: "Test1_#{type}",
+        note: 'Test1 Role.',
+        default_at_signup: false,
+        permissions: [Permission.find_by(name: type)],
+        updated_by_id: 1,
+        created_by_id: 1
+      )
+      assert(role)
+    end
+
+    assert_raises(Exceptions::UnprocessableEntity) do
+      Role.create!(
+        name: 'Test2',
+        note: 'Test2 Role.',
+        default_at_signup: true,
+        permissions: [Permission.find_by(name: 'ticket.agent')],
+        updated_by_id: 1,
+        created_by_id: 1
+      )
+    end
+
+    role = Role.create!(
+      name: 'Test2',
+      note: 'Test2 Role.',
+      default_at_signup: false,
+      permissions: [Permission.find_by(name: 'ticket.agent')],
+      updated_by_id: 1,
+      created_by_id: 1
+    )
+    assert(role)
+
+  end
+
 end