From f0462d4c20c2968b52b5dc6a585f26c0409b4fc4 Mon Sep 17 00:00:00 2001 From: Rolf Schmidt Date: Wed, 7 Oct 2020 14:05:07 +0200 Subject: [PATCH] Maintenance: Provide allow_signup column to define the signup permissions for roles and disable new permissions by default as signup permission. --- app/models/role.rb | 11 ++- db/migrate/20120101000001_create_base.rb | 11 +-- .../20201005113317_role_signup_column.rb | 31 ++++++++ db/seeds/permissions.rb | 77 +++++++++++-------- spec/db/migrate/role_signup_column_spec.rb | 30 ++++++++ 5 files changed, 116 insertions(+), 44 deletions(-) create mode 100644 db/migrate/20201005113317_role_signup_column.rb create mode 100644 spec/db/migrate/role_signup_column_spec.rb diff --git a/app/models/role.rb b/app/models/role.rb index 97a18ef22..a50604ab2 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -222,13 +222,12 @@ returns 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 diff --git a/db/migrate/20120101000001_create_base.rb b/db/migrate/20120101000001_create_base.rb index 1cffcac92..79aaa5330 100644 --- a/db/migrate/20120101000001_create_base.rb +++ b/db/migrate/20120101000001_create_base.rb @@ -128,11 +128,12 @@ class CreateBase < ActiveRecord::Migration[4.2] 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 diff --git a/db/migrate/20201005113317_role_signup_column.rb b/db/migrate/20201005113317_role_signup_column.rb new file mode 100644 index 000000000..adb4cd395 --- /dev/null +++ b/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 diff --git a/db/seeds/permissions.rb b/db/seeds/permissions.rb index 70dda8358..0d81da3bc 100644 --- a/db/seeds/permissions.rb +++ b/db/seeds/permissions.rb @@ -263,75 +263,85 @@ Permission.create_if_not_exists( }, ) 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( }, ) 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', diff --git a/spec/db/migrate/role_signup_column_spec.rb b/spec/db/migrate/role_signup_column_spec.rb new file mode 100644 index 000000000..810a05e70 --- /dev/null +++ b/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