diff --git a/lib/pact_broker/api/resources/webhook.rb b/lib/pact_broker/api/resources/webhook.rb index 1f0e27cf7..4d5ce640a 100644 --- a/lib/pact_broker/api/resources/webhook.rb +++ b/lib/pact_broker/api/resources/webhook.rb @@ -33,7 +33,7 @@ def malformed_request? def from_json if webhook - @webhook = webhook_service.update_by_uuid uuid, new_webhook + @webhook = webhook_service.update_by_uuid uuid, params_with_string_keys response.body = to_json else 404 diff --git a/lib/pact_broker/test/test_data_builder.rb b/lib/pact_broker/test/test_data_builder.rb index c2351f1e5..03b39ab7d 100644 --- a/lib/pact_broker/test/test_data_builder.rb +++ b/lib/pact_broker/test/test_data_builder.rb @@ -228,8 +228,8 @@ def create_webhook parameters = {} params[:events] || [{ name: PactBroker::Webhooks::WebhookEvent::DEFAULT_EVENT_NAME }] end events = event_params.collect{ |e| PactBroker::Webhooks::WebhookEvent.new(e) } - default_params = { method: 'POST', url: 'http://example.org', headers: {'Content-Type' => 'application/json'}, username: params[:username], password: params[:password]} - request = PactBroker::Webhooks::WebhookRequestTemplate.new(default_params.merge(params)) + template_params = { method: 'POST', url: 'http://example.org', headers: {'Content-Type' => 'application/json'}, username: params[:username], password: params[:password]} + request = PactBroker::Webhooks::WebhookRequestTemplate.new(template_params.merge(params)) @webhook = PactBroker::Webhooks::Repository.new.create uuid, PactBroker::Domain::Webhook.new(request: request, events: events), consumer, provider self end diff --git a/lib/pact_broker/webhooks/service.rb b/lib/pact_broker/webhooks/service.rb index 006118b84..b0dd80311 100644 --- a/lib/pact_broker/webhooks/service.rb +++ b/lib/pact_broker/webhooks/service.rb @@ -9,6 +9,7 @@ require 'pact_broker/webhooks/webhook_event' require 'pact_broker/verifications/placeholder_verification' require 'pact_broker/pacts/placeholder_pact' +require 'pact_broker/api/decorators/webhook_decorator' module PactBroker @@ -40,7 +41,16 @@ def self.find_by_uuid uuid webhook_repository.find_by_uuid uuid end - def self.update_by_uuid uuid, webhook + 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 + PactBroker::Api::Decorators::WebhookDecorator.new(webhook).from_hash(params) webhook_repository.update_by_uuid uuid, webhook end diff --git a/spec/lib/pact_broker/webhooks/service_spec.rb b/spec/lib/pact_broker/webhooks/service_spec.rb index 1426bf266..987e37fe5 100644 --- a/spec/lib/pact_broker/webhooks/service_spec.rb +++ b/spec/lib/pact_broker/webhooks/service_spec.rb @@ -33,6 +33,56 @@ module Webhooks end end + describe ".update_by_uuid" do + before do + 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(:existing_webhook) { PactBroker::Domain::Webhook.new(request: request) } + let(:params) do + { + 'request' => { + 'url' => "http://url" + } + } + end + + subject { Service.update_by_uuid("1234", params) } + + context "when the webhook has a password and the incoming parameters do not contain a password" do + 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 "the incoming parameters contain a password" do + let(:params) do + { + 'request' => { + 'password' => "updated" + } + } + end + + it "updates 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 'updated' + end + end + end + describe ".trigger_webhooks" do let(:verification) { instance_double(PactBroker::Domain::Verification)}