feat(webhooks): maintain starred out Authorization header value

Author: bethesque

lib/pact_broker/domain/webhook_request.rb

@@ -9,6 +9,7 @@
 require 'pact_broker/build_http_options'
 require 'cgi'
 require 'delegate'
+require 'rack/utils'
 
 module PactBroker
 

lib/pact_broker/webhooks/service.rb

@@ -43,13 +43,7 @@ def self.find_by_uuid uuid
 
       def self.update_by_uuid uuid, params
         webhook = webhook_repository.find_by_uuid(uuid)
-        # Dirty hack to maintain existing password if it is not submitted
-        # This is required because the password is not returned in the API response
-        # for security purposes, so it needs to be re-entered with every response.
-        # TODO implement proper 'secrets' management.
-        if webhook.request.password && !params['request'].key?('password')
-          params['request']['password'] = webhook.request.password
-        end
+        maintain_redacted_params(webhook, params)
         PactBroker::Api::Decorators::WebhookDecorator.new(webhook).from_hash(params)
         webhook_repository.update_by_uuid uuid, webhook
       end
@@ -171,6 +165,31 @@ def self.find_triggered_webhooks_for_pact pact
       def self.find_triggered_webhooks_for_verification verification
         webhook_repository.find_triggered_webhooks_for_verification(verification)
       end
+
+      private
+
+      # Dirty hack to maintain existing password or Authorization header if it is submitted with value ****
+      # This is required because the password and Authorization header is **** out in the API response
+      # for security purposes, so it would need to be re-entered with every response.
+      # TODO implement proper 'secrets' management.
+      def self.maintain_redacted_params(webhook, params)
+        if webhook.request.password && password_key_does_not_exist_or_is_starred?(params)
+          params['request']['password'] = webhook.request.password
+        end
+
+        new_headers = params['request']['headers'] ||= {}
+        existing_headers = webhook.request.headers
+        starred_new_headers = new_headers.select { |key, value| value =~ /^\**$/ }
+        starred_new_headers.each do | (key, value) |
+          new_headers[key] = existing_headers[key]
+        end
+        params['request']['headers'] = new_headers
+        params
+      end
+
+      def self.password_key_does_not_exist_or_is_starred?(params)
+        !params['request'].key?('password') || params.dig('request','password') =~ /^\**$/
+      end
     end
   end
 end

lib/pact_broker/webhooks/webhook_request_template.rb

@@ -22,7 +22,7 @@ def initialize attributes = {}
         @url = attributes[:url]
         @username = attributes[:username]
         @password = attributes[:password]
-        @headers = attributes[:headers] || {}
+        @headers = Rack::Utils::HeaderHash.new(attributes[:headers] || {})
         @body = attributes[:body]
         @uuid = attributes[:uuid]
       end
@@ -64,6 +64,10 @@ def redacted_headers
         end
       end
 
+      def headers= headers
+        @headers = Rack::Utils::HeaderHash.new(headers)
+      end
+
       private
 
       def to_s

spec/lib/pact_broker/webhooks/service_spec.rb

@@ -38,7 +38,9 @@ module Webhooks
           allow_any_instance_of(PactBroker::Webhooks::Repository).to receive(:find_by_uuid).and_return(existing_webhook)
         end
 
-        let(:request) { PactBroker::Webhooks::WebhookRequestTemplate.new(password: 'password')}
+        let(:request) { PactBroker::Webhooks::WebhookRequestTemplate.new(password: existing_password, headers: headers)}
+        let(:existing_password) { nil }
+        let(:headers) { {} }
         let(:existing_webhook) { PactBroker::Domain::Webhook.new(request: request) }
         let(:params) do
           {
@@ -50,7 +52,19 @@ module Webhooks
 
         subject { Service.update_by_uuid("1234", params) }
 
+        it "sends through the params to the repository" do
+          updated_webhook = nil
+          allow_any_instance_of(PactBroker::Webhooks::Repository).to receive(:update_by_uuid) do | instance, uuid, webhook |
+            updated_webhook = webhook
+            true
+          end
+          subject
+          expect(updated_webhook.request.url).to eq 'http://url'
+        end
+
         context "when the webhook has a password and the incoming parameters do not contain a password" do
+          let(:existing_password) { 'password' }
+
           it "does not overwite the password" do
             updated_webhook = nil
             allow_any_instance_of(PactBroker::Webhooks::Repository).to receive(:update_by_uuid) do | instance, uuid, webhook |
@@ -62,6 +76,52 @@ module Webhooks
           end
         end
 
+        context "when the webhook has a password and the incoming parameters contain a *** password" do
+          let(:existing_password) { 'password' }
+          let(:params) do
+            {
+              'request' => {
+                'url' => 'http://url',
+                'password' => '*******'
+              }
+            }
+          end
+
+          it "does not overwite the password" do
+            updated_webhook = nil
+            allow_any_instance_of(PactBroker::Webhooks::Repository).to receive(:update_by_uuid) do | instance, uuid, webhook |
+              updated_webhook = webhook
+              true
+            end
+            subject
+            expect(updated_webhook.request.password).to eq 'password'
+          end
+        end
+
+        context "when the webhook has an authorization header and the incoming parameters contain a *** authorization header" do
+          let(:headers) { { 'Authorization' => 'existing'} }
+          let(:params) do
+            {
+              'request' => {
+                'url' => "http://url",
+                'headers' => {
+                  'authorization' => "***********"
+                }
+              }
+            }
+          end
+
+          it "does not overwite the authorization header" do
+            updated_webhook = nil
+            allow_any_instance_of(PactBroker::Webhooks::Repository).to receive(:update_by_uuid) do | instance, uuid, webhook |
+              updated_webhook = webhook
+              true
+            end
+            subject
+            expect(updated_webhook.request.headers['Authorization']).to eq 'existing'
+          end
+        end
+
         context "the incoming parameters contain a password" do
           let(:params) do
             {