From 9423478a86c407973578df6ec72cabc58b4474ec Mon Sep 17 00:00:00 2001 From: James Wong Date: Fri, 12 May 2017 14:33:55 -0400 Subject: [PATCH 1/2] encry secretes before enqueue for Tower CUD --- .../shared/automation_manager/credential.rb | 8 +++++--- .../ansible_tower/shared/automation_manager/tower_api.rb | 7 +++++-- lib/vmdb/settings/walker.rb | 9 ++++++++- .../ansible_shared/automation_manager/credential.rb | 6 ++++-- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/app/models/manageiq/providers/ansible_tower/shared/automation_manager/credential.rb b/app/models/manageiq/providers/ansible_tower/shared/automation_manager/credential.rb index e356aaaff6a..a4a8236928a 100644 --- a/app/models/manageiq/providers/ansible_tower/shared/automation_manager/credential.rb +++ b/app/models/manageiq/providers/ansible_tower/shared/automation_manager/credential.rb @@ -16,9 +16,11 @@ def provider_params(params) params end - def hide_secrets(params) - params.each_with_object({}) do |attr, h| - h[attr.first] = self::API_ATTRIBUTES[attr.first] && self::API_ATTRIBUTES[attr.first][:type] == :password ? '******' : attr.second + def process_secrets(params, decrypt = false) + if decrypt + Vmdb::Settings::decrypt_passwords!(params) + else + Vmdb::Settings::encrypt_passwords!(params) end end end diff --git a/app/models/manageiq/providers/ansible_tower/shared/automation_manager/tower_api.rb b/app/models/manageiq/providers/ansible_tower/shared/automation_manager/tower_api.rb index 33c2dbf7771..fece8fc2a3d 100644 --- a/app/models/manageiq/providers/ansible_tower/shared/automation_manager/tower_api.rb +++ b/app/models/manageiq/providers/ansible_tower/shared/automation_manager/tower_api.rb @@ -4,6 +4,7 @@ module ManageIQ::Providers::AnsibleTower::Shared::AutomationManager::TowerApi module ClassMethods def create_in_provider(manager_id, params) params = provider_params(params) if respond_to?(:provider_params) + process_secrets(params, true) if respond_to?(:process_secrets) manager = ExtManagementSystem.find(manager_id) tower_object = provider_collection(manager).create!(params) if respond_to?(:refresh_in_provider) @@ -18,6 +19,7 @@ def create_in_provider(manager_id, params) end def create_in_provider_queue(manager_id, params) + process_secrets(params) if respond_to?(:process_secrets) manager = ExtManagementSystem.find(manager_id) action = "Creating #{self::FRIENDLY_NAME} (name=#{params[:name]})" queue(manager.my_zone, nil, "create_in_provider", [manager_id, params], action) @@ -25,9 +27,8 @@ def create_in_provider_queue(manager_id, params) private def notify(op, manager_id, params, success) - params = hide_secrets(params) if respond_to?(:hide_secrets) - _log.info "#{name} in_provider #{op} with parameters: #{params} #{success ? 'succeeded' : 'failed'}" op_arg = params.each_with_object([]) { |(k, v), l| l.push("#{k}=#{v}") if [:name, :manager_ref].include?(k) }.join(', ') + _log.info "#{name} in_provider #{op} with parameters: #{op_arg} #{success ? 'succeeded' : 'failed'}" Notification.create( :type => success ? :tower_op_success : :tower_op_failure, :options => { @@ -64,6 +65,7 @@ def queue(zone, instance_id, method_name, args, action) end def update_in_provider(params) + self.class.process_secrets(params, true) if self.class.respond_to?(:process_secrets) params.delete(:task_id) # in case this is being called through update_in_provider_queue which will stick in a :task_id params = self.class.provider_params(params) if self.class.respond_to?(:provider_params) with_provider_object do |provider_object| @@ -81,6 +83,7 @@ def update_in_provider(params) end def update_in_provider_queue(params) + self.class.process_secrets(params) if self.class.respond_to?(:process_secrets) action = "Updating #{self.class::FRIENDLY_NAME} (Tower internal reference=#{manager_ref})" self.class.send('queue', manager.my_zone, id, "update_in_provider", [params], action) end diff --git a/lib/vmdb/settings/walker.rb b/lib/vmdb/settings/walker.rb index a9b2322c309..30505f4a264 100644 --- a/lib/vmdb/settings/walker.rb +++ b/lib/vmdb/settings/walker.rb @@ -1,7 +1,14 @@ module Vmdb class Settings module Walker - PASSWORD_FIELDS = %i(bind_pwd password amazon_secret).to_set.freeze + def self.secret_set + Set.new([:password, :ssh_key_data, :ssh_key_unlock, :become_password, :vault_password, :security_token]) + # ManageIQ::Providers::EmbeddedAnsible::AutomationManager::Credential.descendants.inject(Set.new) do |s, klass| + # s += klass::API_ATTRIBUTES.collect{ |k, meta| k if meta[:type]==:password }.compact + # end + end + + PASSWORD_FIELDS = (%i(bind_pwd password amazon_secret).to_set + secret_set).freeze # Walks the settings and yields each value along the way # diff --git a/spec/support/ansible_shared/automation_manager/credential.rb b/spec/support/ansible_shared/automation_manager/credential.rb index 53686fbd1d7..184a5ebcd59 100644 --- a/spec/support/ansible_shared/automation_manager/credential.rb +++ b/spec/support/ansible_shared/automation_manager/credential.rb @@ -175,11 +175,13 @@ def store_new_credential(credential, manager) end it "#update_in_provider_queue" do - task_id = ansible_cred.update_in_provider_queue({}) + expect(Vmdb::Settings).to receive(:encrypt_passwords!).with(params) + task_id = ansible_cred.update_in_provider_queue(params) expect(MiqTask.find(task_id)).to have_attributes(:name => "Updating #{described_class::FRIENDLY_NAME} (Tower internal reference=#{ansible_cred.manager_ref})") + params[:task_id] = task_id expect(MiqQueue.first).to have_attributes( :instance_id => ansible_cred.id, - :args => [{:task_id => task_id}], + :args => [params], :class_name => described_class.name, :method_name => "update_in_provider", :priority => MiqQueue::HIGH_PRIORITY, From 48704ac60d138663512b924acf647af6089b7930 Mon Sep 17 00:00:00 2001 From: James Wong Date: Fri, 12 May 2017 14:35:49 -0400 Subject: [PATCH 2/2] simply add tokens to PASSWORD_FIELDS https://bugzilla.redhat.com/show_bug.cgi?id=1450183 --- .../shared/automation_manager/credential.rb | 4 ++-- .../ansible_tower/shared/automation_manager/tower_api.rb | 1 + lib/vmdb/settings/walker.rb | 9 +-------- .../ansible_shared/automation_manager/credential.rb | 4 ++++ 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/app/models/manageiq/providers/ansible_tower/shared/automation_manager/credential.rb b/app/models/manageiq/providers/ansible_tower/shared/automation_manager/credential.rb index a4a8236928a..e7c831ff02e 100644 --- a/app/models/manageiq/providers/ansible_tower/shared/automation_manager/credential.rb +++ b/app/models/manageiq/providers/ansible_tower/shared/automation_manager/credential.rb @@ -18,9 +18,9 @@ def provider_params(params) def process_secrets(params, decrypt = false) if decrypt - Vmdb::Settings::decrypt_passwords!(params) + Vmdb::Settings.decrypt_passwords!(params) else - Vmdb::Settings::encrypt_passwords!(params) + Vmdb::Settings.encrypt_passwords!(params) end end end diff --git a/app/models/manageiq/providers/ansible_tower/shared/automation_manager/tower_api.rb b/app/models/manageiq/providers/ansible_tower/shared/automation_manager/tower_api.rb index fece8fc2a3d..2fdd427a638 100644 --- a/app/models/manageiq/providers/ansible_tower/shared/automation_manager/tower_api.rb +++ b/app/models/manageiq/providers/ansible_tower/shared/automation_manager/tower_api.rb @@ -26,6 +26,7 @@ def create_in_provider_queue(manager_id, params) end private + def notify(op, manager_id, params, success) op_arg = params.each_with_object([]) { |(k, v), l| l.push("#{k}=#{v}") if [:name, :manager_ref].include?(k) }.join(', ') _log.info "#{name} in_provider #{op} with parameters: #{op_arg} #{success ? 'succeeded' : 'failed'}" diff --git a/lib/vmdb/settings/walker.rb b/lib/vmdb/settings/walker.rb index 30505f4a264..bda079d2122 100644 --- a/lib/vmdb/settings/walker.rb +++ b/lib/vmdb/settings/walker.rb @@ -1,14 +1,7 @@ module Vmdb class Settings module Walker - def self.secret_set - Set.new([:password, :ssh_key_data, :ssh_key_unlock, :become_password, :vault_password, :security_token]) - # ManageIQ::Providers::EmbeddedAnsible::AutomationManager::Credential.descendants.inject(Set.new) do |s, klass| - # s += klass::API_ATTRIBUTES.collect{ |k, meta| k if meta[:type]==:password }.compact - # end - end - - PASSWORD_FIELDS = (%i(bind_pwd password amazon_secret).to_set + secret_set).freeze + PASSWORD_FIELDS = %i(bind_pwd password amazon_secret ssh_key_data ssh_key_unlock become_password vault_password security_token).to_set.freeze # Walks the settings and yields each value along the way # diff --git a/spec/support/ansible_shared/automation_manager/credential.rb b/spec/support/ansible_shared/automation_manager/credential.rb index 184a5ebcd59..884cad0b13c 100644 --- a/spec/support/ansible_shared/automation_manager/credential.rb +++ b/spec/support/ansible_shared/automation_manager/credential.rb @@ -52,6 +52,7 @@ it ".create_in_provider to succeed and send notification" do expected_params[:organization] = 1 if described_class.name.include?("::EmbeddedAnsible::") + expect(Vmdb::Settings).to receive(:decrypt_passwords!).with(expected_params) expect(AnsibleTowerClient::Connection).to receive(:new).and_return(atc) store_new_credential(credential, manager) expect(EmsRefresh).to receive(:queue_refresh_task).with(manager).and_return([finished_task]) @@ -63,6 +64,7 @@ it ".create_in_provider to fail (not found during refresh) and send notification" do expected_params[:organization] = 1 if described_class.name.include?("::EmbeddedAnsible::") + expect(Vmdb::Settings).to receive(:decrypt_passwords!).with(expected_params) expect(AnsibleTowerClient::Connection).to receive(:new).and_return(atc) expect(EmsRefresh).to receive(:queue_refresh_task).and_return([finished_task]) expect(ExtManagementSystem).to receive(:find).with(manager.id).and_return(manager) @@ -73,6 +75,7 @@ end it ".create_in_provider_queue" do + expect(Vmdb::Settings).to receive(:encrypt_passwords!).with(params) task_id = described_class.create_in_provider_queue(manager.id, params) expect(MiqTask.find(task_id)).to have_attributes(:name => "Creating #{described_class::FRIENDLY_NAME} (name=#{params[:name]})") expect(MiqQueue.first).to have_attributes( @@ -157,6 +160,7 @@ def store_new_credential(credential, manager) end it "#update_in_provider to succeed and send notification" do + expect(Vmdb::Settings).to receive(:decrypt_passwords!).with(params) expected_params[:organization] = 1 if described_class.name.include?("::EmbeddedAnsible::") expect(AnsibleTowerClient::Connection).to receive(:new).and_return(atc) expect(credential).to receive(:update_attributes!).with(expected_params)