From 9472af70f119c31c1d2623916860681c8b4dc2b1 Mon Sep 17 00:00:00 2001
From: Rolf Schmidt <rolf.schmidt@znuny.com>
Date: Sat, 18 Jan 2020 12:01:51 +0100
Subject: [PATCH] Fixed issue #2893 - Granted field access gets redacted by a
 later (alphabetically) permission.

---
 app/models/object_manager/attribute.rb       | 21 ++++++++--
 spec/models/object_manager/attribute_spec.rb | 44 ++++++++++++++++++++
 2 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/app/models/object_manager/attribute.rb b/app/models/object_manager/attribute.rb
index 2596c5a2e..40fa9c00a 100644
--- a/app/models/object_manager/attribute.rb
+++ b/app/models/object_manager/attribute.rb
@@ -508,11 +508,24 @@ returns:
         data[:screen] = {}
         item.screens.each do |screen, permission_options|
           data[:screen][screen] = {}
+
+          if permission_options['-all-']
+            data[:screen][screen] = permission_options['-all-']
+            next
+          end
+
           permission_options.each do |permission, options|
-            if permission == '-all-'
-              data[:screen][screen] = options
-            elsif user&.permissions?(permission)
-              data[:screen][screen] = options
+            next if !user&.permissions?(permission)
+
+            options.each do |key, value|
+              if [true, false].include?(data[:screen][screen][key])
+                data[:screen][screen][key] = data[:screen][screen][key].nil? ? false : data[:screen][screen][key]
+                if options[key]
+                  data[:screen][screen][key] = true
+                end
+              else
+                data[:screen][screen][key] = value
+              end
             end
           end
         end
diff --git a/spec/models/object_manager/attribute_spec.rb b/spec/models/object_manager/attribute_spec.rb
index 523b6da4c..3e973fac0 100644
--- a/spec/models/object_manager/attribute_spec.rb
+++ b/spec/models/object_manager/attribute_spec.rb
@@ -1,6 +1,18 @@
 require 'rails_helper'
 
 RSpec.describe ObjectManager::Attribute, type: :model do
+
+  let(:user_attribute_permissions) do
+    create(:user, roles: [role_attribute_permissions])
+  end
+
+  let(:role_attribute_permissions) do
+    create(:role).tap do |role|
+      role.permission_grant('admin.organization')
+      role.permission_grant('ticket.agent')
+    end
+  end
+
   describe 'callbacks' do
     context 'for setting default values on local data options' do
       let(:subject) { described_class.new }
@@ -106,4 +118,36 @@ RSpec.describe ObjectManager::Attribute, type: :model do
       end.not_to raise_error
     end
   end
+
+  describe 'attribute permissions', db_strategy: :reset do
+    it 'merges attribute permissions' do
+      create(:object_manager_attribute_text, screens: { create: { 'admin.organization': { shown: true }, 'ticket.agent': { shown: false } } }, name: 'test_permissions')
+
+      migration = described_class.migration_execute
+      expect(migration).to be true
+
+      attribute = described_class.by_object('Ticket', user_attribute_permissions).detect { |attr| attr[:name] == 'test_permissions' }
+      expect(attribute[:screen]['create']['shown']).to be true
+    end
+
+    it 'overwrites permissions if all get set' do
+      create(:object_manager_attribute_text, screens: { create: { '-all-': { shown: true }, 'admin.organization': { shown: false }, 'ticket.agent': { shown: false } } }, name: 'test_permissions_all')
+
+      migration = described_class.migration_execute
+      expect(migration).to be true
+
+      attribute = described_class.by_object('Ticket', user_attribute_permissions).detect { |attr| attr[:name] == 'test_permissions_all' }
+      expect(attribute[:screen]['create']['shown']).to be true
+    end
+
+    it 'is able to handle other values than true or false' do
+      create(:object_manager_attribute_text, screens: { create: { '-all-': { shown: true, item_class: 'column' }, 'admin.organization': { shown: false }, 'ticket.agent': { shown: false } } }, name: 'test_permissions_item')
+
+      migration = described_class.migration_execute
+      expect(migration).to be true
+
+      attribute = described_class.by_object('Ticket', user_attribute_permissions).detect { |attr| attr[:name] == 'test_permissions_item' }
+      expect(attribute[:screen]['create']['item_class']).to eq('column')
+    end
+  end
 end