Maintenance: Provide allow_signup column to define the signup permiss…

…ions for roles and disable new permissions by default as signup permission.

app/models/role.rb

@@ -222,13 +222,12 @@ def validate_agent_limit_by_permission(permission)
   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
+    return true if !default_at_signup
 
-    raise Exceptions::UnprocessableEntity, 'Cannot set default at signup when role has admin or ticket.agent permissions.'
+    forbidden_permissions = permissions.reject(&:allow_signup)
+    return true if forbidden_permissions.blank?
+
+    raise Exceptions::UnprocessableEntity, "Cannot set default at signup when role has #{forbidden_permissions.join(', ')} permissions."
   end
 
 end

db/migrate/20120101000001_create_base.rb

@@ -128,11 +128,12 @@ def up
     add_foreign_key :roles, :users, column: :updated_by_id
 
     create_table :permissions do |t|
-      t.string :name,                   limit: 255, null: false
-      t.string :note,                   limit: 500, null: true
-      t.string :preferences,            limit: 10_000, null: true
-      t.boolean :active,                               null: false, default: true
-      t.timestamps limit: 3, null: false
+      t.string :name,          limit: 255, null: false
+      t.string :note,          limit: 500, null: true
+      t.string :preferences,   limit: 10_000, null: true
+      t.boolean :active,       null: false, default: true
+      t.boolean :allow_signup, null: false, default: false
+      t.timestamps limit: 3,   null: false
     end
     add_index :permissions, [:name], unique: true
 

db/migrate/20201005113317_role_signup_column.rb

@@ -0,0 +1,31 @@
+class RoleSignupColumn < ActiveRecord::Migration[5.2]
+  def change
+
+    # return if it's a new setup
+    return if !Setting.exists?(name: 'system_init_done')
+
+    add_column :permissions, :allow_signup, :boolean, null: false, default: false
+
+    signup_permissions = [
+      'user_preferences',
+      'user_preferences.password',
+      'user_preferences.notifications',
+      'user_preferences.access_token',
+      'user_preferences.language',
+      'user_preferences.linked_accounts',
+      'user_preferences.device',
+      'user_preferences.avatar',
+      'user_preferences.calendar',
+      'user_preferences.out_of_office',
+      'ticket.customer',
+    ]
+
+    Permission.where(name: signup_permissions).update(allow_signup: true)
+
+    Role.where(default_at_signup: true).find_each do |role|
+      role.permissions.where.not(name: signup_permissions).find_each do |permission|
+        role.permission_revoke(permission.name)
+      end
+    end
+  end
+end

db/seeds/permissions.rb

@@ -263,75 +263,85 @@
   },
 )
 Permission.create_if_not_exists(
-  name:        'user_preferences',
-  note:        'User Preferences',
-  preferences: {},
+  name:         'user_preferences',
+  note:         'User Preferences',
+  preferences:  {},
+  allow_signup: true,
 )
 Permission.create_if_not_exists(
-  name:        'user_preferences.password',
-  note:        'Change %s',
-  preferences: {
+  name:         'user_preferences.password',
+  note:         'Change %s',
+  preferences:  {
     translations: ['Password']
   },
+  allow_signup: true,
 )
 Permission.create_if_not_exists(
-  name:        'user_preferences.notifications',
-  note:        'Manage %s',
-  preferences: {
+  name:         'user_preferences.notifications',
+  note:         'Manage %s',
+  preferences:  {
     translations: ['Notifications'],
     required:     ['ticket.agent'],
   },
+  allow_signup: true,
 )
 Permission.create_if_not_exists(
-  name:        'user_preferences.access_token',
-  note:        'Manage %s',
-  preferences: {
+  name:         'user_preferences.access_token',
+  note:         'Manage %s',
+  preferences:  {
     translations: ['Token Access']
   },
+  allow_signup: true,
 )
 Permission.create_if_not_exists(
-  name:        'user_preferences.language',
-  note:        'Change %s',
-  preferences: {
+  name:         'user_preferences.language',
+  note:         'Change %s',
+  preferences:  {
     translations: ['Language']
   },
+  allow_signup: true,
 )
 Permission.create_if_not_exists(
-  name:        'user_preferences.linked_accounts',
-  note:        'Manage %s',
-  preferences: {
+  name:         'user_preferences.linked_accounts',
+  note:         'Manage %s',
+  preferences:  {
     translations: ['Linked Accounts']
   },
+  allow_signup: true,
 )
 Permission.create_if_not_exists(
-  name:        'user_preferences.device',
-  note:        'Manage %s',
-  preferences: {
+  name:         'user_preferences.device',
+  note:         'Manage %s',
+  preferences:  {
     translations: ['Devices']
   },
+  allow_signup: true,
 )
 Permission.create_if_not_exists(
-  name:        'user_preferences.avatar',
-  note:        'Manage %s',
-  preferences: {
+  name:         'user_preferences.avatar',
+  note:         'Manage %s',
+  preferences:  {
     translations: ['Avatar']
   },
+  allow_signup: true,
 )
 Permission.create_if_not_exists(
-  name:        'user_preferences.calendar',
-  note:        'Access to %s',
-  preferences: {
+  name:         'user_preferences.calendar',
+  note:         'Access to %s',
+  preferences:  {
     translations: ['Calendars'],
     required:     ['ticket.agent'],
   },
+  allow_signup: true,
 )
 Permission.create_if_not_exists(
-  name:        'user_preferences.out_of_office',
-  note:        'Change %s',
-  preferences: {
+  name:         'user_preferences.out_of_office',
+  note:         'Change %s',
+  preferences:  {
     translations: ['Out of Office'],
     required:     ['ticket.agent'],
   },
+  allow_signup: true,
 )
 
 Permission.create_if_not_exists(
@@ -354,9 +364,10 @@
   },
 )
 Permission.create_if_not_exists(
-  name:        'ticket.customer',
-  note:        'Access to Customer Tickets based on current_user and organization',
-  preferences: {},
+  name:         'ticket.customer',
+  note:         'Access to Customer Tickets based on current_user and organization',
+  preferences:  {},
+  allow_signup: true,
 )
 Permission.create_if_not_exists(
   name:        'chat',

spec/db/migrate/role_signup_column_spec.rb

@@ -0,0 +1,30 @@
+require 'rails_helper'
+
+RSpec.describe RoleSignupColumn, type: :db_migration, db_strategy: :reset do
+  context 'when a role contains signup permissions' do
+    let!(:role) do
+      role = create(:role)
+      role.permission_grant('user_preferences.password')
+      role.permission_grant('ticket.agent')
+      role.update_column(:default_at_signup, true)
+      role
+    end
+
+    before do
+      without_column(:permissions, column: :allow_signup)
+      migrate
+    end
+
+    it 'has password permission' do
+      expect(role.reload.permissions.map(&:name)).to include('user_preferences.password')
+    end
+
+    it 'has no agent permission' do
+      expect(role.reload.permissions.map(&:name)).not_to include('ticket.agent')
+    end
+
+    it 'has permission with allow_signup set correctly' do
+      expect(Permission.find_by(name: 'user_preferences.password').allow_signup).to be true
+    end
+  end
+end