diff --git a/app/assets/javascripts/webhooks/components/form_custom_header_item.vue b/app/assets/javascripts/webhooks/components/form_custom_header_item.vue new file mode 100644 index 0000000000000000000000000000000000000000..4ab18a01a9b485b7d9e45bfcd6c51ee7826aa1aa --- /dev/null +++ b/app/assets/javascripts/webhooks/components/form_custom_header_item.vue @@ -0,0 +1,115 @@ + + + diff --git a/app/assets/javascripts/webhooks/components/form_custom_headers.vue b/app/assets/javascripts/webhooks/components/form_custom_headers.vue new file mode 100644 index 0000000000000000000000000000000000000000..4791b69b3f56854b48810610ada7bb62576f8502 --- /dev/null +++ b/app/assets/javascripts/webhooks/components/form_custom_headers.vue @@ -0,0 +1,149 @@ + + + diff --git a/app/assets/javascripts/webhooks/components/webhook_form_app.vue b/app/assets/javascripts/webhooks/components/webhook_form_app.vue new file mode 100644 index 0000000000000000000000000000000000000000..8b6527ee2dd159d8b011ddcc0931239d5f3a085a --- /dev/null +++ b/app/assets/javascripts/webhooks/components/webhook_form_app.vue @@ -0,0 +1,40 @@ + + + diff --git a/app/assets/javascripts/webhooks/constants.js b/app/assets/javascripts/webhooks/constants.js index 96632b47e6b8c742e7e0204fdc8f7d88a5ef27a7..94b176e5ea00c55c8c8f4b1f201d8c66f65f25d2 100644 --- a/app/assets/javascripts/webhooks/constants.js +++ b/app/assets/javascripts/webhooks/constants.js @@ -17,3 +17,5 @@ export const descriptionText = { }; export const MASK_ITEM_VALUE_HIDDEN = '************'; + +export const CUSTOM_HEADER_KEY_PATTERN = /^[A-Za-z]+[0-9]*(?:[._-][A-Za-z0-9]+)*$/; diff --git a/app/assets/javascripts/webhooks/index.js b/app/assets/javascripts/webhooks/index.js index 6eb7cbea72cf3e457f704ec83fd5487c64d0d77a..ca4152d03a0e8a2e83cc331818feb0a46fd5a32c 100644 --- a/app/assets/javascripts/webhooks/index.js +++ b/app/assets/javascripts/webhooks/index.js @@ -1,5 +1,5 @@ import Vue from 'vue'; -import FormUrlApp from './components/form_url_app.vue'; +import WebhookFormApp from './components/webhook_form_app.vue'; import TestDropdown from './components/test_dropdown.vue'; export default () => { @@ -9,16 +9,17 @@ export default () => { return null; } - const { url: initialUrl, urlVariables } = el.dataset; + const { url: initialUrl, urlVariables, customHeaders } = el.dataset; return new Vue({ el, name: 'WebhookFormRoot', render(createElement) { - return createElement(FormUrlApp, { + return createElement(WebhookFormApp, { props: { initialUrl, initialUrlVariables: JSON.parse(urlVariables), + initialCustomHeaders: JSON.parse(customHeaders), }, }); }, diff --git a/app/controllers/concerns/web_hooks/hook_actions.rb b/app/controllers/concerns/web_hooks/hook_actions.rb index aae38e67e23008afd25cf5a0cfe4b7339f63e352..e3a88487e4af20936abc0c4dd74f0c9ad9692de6 100644 --- a/app/controllers/concerns/web_hooks/hook_actions.rb +++ b/app/controllers/concerns/web_hooks/hook_actions.rb @@ -10,6 +10,14 @@ module HookActions before_action :hook_logs, only: :edit feature_category :webhooks + + before_action only: %i[edit update] do + push_frontend_feature_flag(:custom_webhook_headers, hook.parent, type: :beta) + end + + before_action only: :index do + push_frontend_feature_flag(:custom_webhook_headers, @project || @group, type: :beta) + end end def index @@ -54,13 +62,14 @@ def edit def hook_params permitted = hook_param_names + trigger_values - permitted << { url_variables: [:key, :value] } + permitted << { url_variables: [:key, :value], custom_headers: [:key, :value] } ps = params.require(:hook).permit(*permitted).to_h ps.delete(:token) if action_name == 'update' && ps[:token] == WebHook::SECRET_MASK ps[:url_variables] = ps[:url_variables].to_h { [_1[:key], _1[:value].presence] } if ps.key?(:url_variables) + ps[:custom_headers] = ps[:custom_headers].to_h { [_1[:key], hook_value_from_param_or_db(_1[:key], _1[:value])] } if action_name == 'update' && ps.key?(:url_variables) supplied = ps[:url_variables] @@ -88,5 +97,13 @@ def destroy_hook(hook) def hook_logs @hook_logs ||= hook.web_hook_logs.recent.page(params[:page]).without_count end + + def hook_value_from_param_or_db(key, value) + if value == WebHook::SECRET_MASK && hook.custom_headers.key?(key) + hook.custom_headers[key] + else + value + end + end end end diff --git a/app/helpers/hooks_helper.rb b/app/helpers/hooks_helper.rb index ac1e4456bc7c1263b1b2e2a9041a2a46d5c003e8..cbab251f3af3d5ef011e623010b50b64555ea5d1 100644 --- a/app/helpers/hooks_helper.rb +++ b/app/helpers/hooks_helper.rb @@ -4,7 +4,8 @@ module HooksHelper def webhook_form_data(hook) { url: hook.url, - url_variables: Gitlab::Json.dump(hook.url_variables.keys.map { { key: _1 } }) + url_variables: Gitlab::Json.dump(hook.url_variables.keys.map { { key: _1 } }), + custom_headers: Gitlab::Json.dump(hook.custom_headers.keys.map { { key: _1, value: WebHook::SECRET_MASK } }) } end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index 24f19b932e0e747657180add2ae4ef038c56d0bc..13ef26877d5f8c07159e63fc7d0d4f6352ec51e6 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -27,6 +27,15 @@ class WebHook < ApplicationRecord encode: false, encode_iv: false + attr_encrypted :custom_headers, + mode: :per_attribute_iv, + key: Settings.attr_encrypted_db_key_base_32, + algorithm: 'aes-256-gcm', + marshal: true, + marshaler: ::Gitlab::Json, + encode: false, + encode_iv: false + has_many :web_hook_logs validates :url, presence: true @@ -34,9 +43,11 @@ class WebHook < ApplicationRecord validates :token, format: { without: /\n/ } after_initialize :initialize_url_variables + after_initialize :initialize_custom_headers before_validation :reset_token before_validation :reset_url_variables, unless: ->(hook) { hook.is_a?(ServiceHook) }, on: :update + before_validation :reset_custom_headers, unless: ->(hook) { hook.is_a?(ServiceHook) }, on: :update before_validation :set_branch_filter_nil, if: :branch_filter_strategy_all_branches? validates :push_events_branch_filter, untrusted_regexp: true, if: :branch_filter_strategy_regex? validates :push_events_branch_filter, "web_hooks/wildcard_branch_filter": true, if: :branch_filter_strategy_wildcard? @@ -44,6 +55,7 @@ class WebHook < ApplicationRecord validates :url_variables, json_schema: { filename: 'web_hooks_url_variables' } validate :no_missing_url_variables validates :interpolated_url, public_url: true, if: ->(hook) { hook.url_variables? && hook.errors.empty? } + validates :custom_headers, json_schema: { filename: 'web_hooks_custom_headers' } validates :custom_webhook_template, length: { maximum: 4096 } enum branch_filter_strategy: { @@ -132,14 +144,27 @@ def reset_token end def reset_url_variables - interpolated_url_was = interpolated_url(decrypt_url_was, url_variables_were) - - return if url_variables_were.blank? || interpolated_url_was == interpolated_url + return if url_variables_were.blank? || !interpolated_url_changed? self.url_variables = {} if url_variables_were.keys.intersection(url_variables.keys).any? self.url_variables = {} if url_changed? && url_variables_were.to_a.intersection(url_variables.to_a).any? end + def reset_custom_headers + return if url.nil? # checking interpolated_url with a nil url causes errors + return unless interpolated_url_changed? + + self.custom_headers = {} + rescue InterpolationError + # ignore -- record is invalid and won't be saved. no need to reset custom_headers + end + + def interpolated_url_changed? + interpolated_url_was = interpolated_url(decrypt_url_was, url_variables_were) + + interpolated_url_was != interpolated_url + end + def decrypt_url_was self.class.decrypt_url(encrypted_url_was, iv: Base64.decode64(encrypted_url_iv_was)) end @@ -152,6 +177,10 @@ def initialize_url_variables self.url_variables = {} if encrypted_url_variables.nil? end + def initialize_custom_headers + self.custom_headers = {} if encrypted_custom_headers.nil? + end + def rate_limiter @rate_limiter ||= Gitlab::WebHooks::RateLimiter.new(self) end diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 2373e1de3dd8074943c3795f1b2a5788410d4261..e89c87b9a8c3b2c8d0ecd9b2e44841edd08d6443 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -138,7 +138,7 @@ def parsed_url def make_request(url, basic_auth = false) Gitlab::HTTP.post(url, body: Gitlab::Json::LimitedEncoder.encode(request_payload, limit: REQUEST_BODY_SIZE_LIMIT), - headers: build_headers, + headers: build_custom_headers.merge(build_headers), verify: hook.enable_ssl_verification, basic_auth: basic_auth, **request_options) @@ -160,7 +160,7 @@ def log_execution(response:, execution_duration:, error_message: nil, request_da url: hook.url, interpolated_url: hook.interpolated_url, execution_duration: execution_duration, - request_headers: build_headers, + request_headers: build_custom_headers(values_redacted: true).merge(build_headers), request_data: request_data, response_headers: safe_response_headers(response), response_body: safe_response_body(response), @@ -216,6 +216,15 @@ def build_headers end end + def build_custom_headers(values_redacted: false) + return {} unless hook.custom_headers.present? + return {} unless Feature.enabled?(:custom_webhook_headers, hook.parent, type: :beta) + + return hook.custom_headers.transform_values { '[REDACTED]' } if values_redacted + + hook.custom_headers + end + # Make response headers more stylish # Net::HTTPHeader has downcased hash with arrays: { 'content-type' => ['text/html; charset=utf-8'] } # This method format response to capitalized hash with strings: { 'Content-Type' => 'text/html; charset=utf-8' } diff --git a/app/validators/json_schemas/web_hooks_custom_headers.json b/app/validators/json_schemas/web_hooks_custom_headers.json new file mode 100644 index 0000000000000000000000000000000000000000..fe69727a172f6ce3322af1a7fed8f98f63891780 --- /dev/null +++ b/app/validators/json_schemas/web_hooks_custom_headers.json @@ -0,0 +1,14 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "WebHook#custom_headers", + "type": "object", + "additionalProperties": false, + "maxProperties": 20, + "patternProperties": { + "^[A-Za-z]+[0-9]*(?:[._-][A-Za-z0-9]+)*$": { + "type": "string", + "minLength": 1, + "maxLength": 2048 + } + } +} diff --git a/config/feature_flags/beta/custom_webhook_headers.yml b/config/feature_flags/beta/custom_webhook_headers.yml new file mode 100644 index 0000000000000000000000000000000000000000..18b36f173fd8fbc0e124a8d15bbae10182478d98 --- /dev/null +++ b/config/feature_flags/beta/custom_webhook_headers.yml @@ -0,0 +1,9 @@ +--- +name: custom_webhook_headers +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/17290 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/146702 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/448604 +milestone: '16.11' +group: group::import and integrate +type: beta +default_enabled: false diff --git a/db/migrate/20240305201830_add_custom_headers_to_web_hook.rb b/db/migrate/20240305201830_add_custom_headers_to_web_hook.rb new file mode 100644 index 0000000000000000000000000000000000000000..4bf4fef0453c7a804ab9b240abd4578a6a6eeb37 --- /dev/null +++ b/db/migrate/20240305201830_add_custom_headers_to_web_hook.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddCustomHeadersToWebHook < Gitlab::Database::Migration[2.2] + enable_lock_retries! + milestone '16.11' + + def change + add_column :web_hooks, :encrypted_custom_headers, :binary + add_column :web_hooks, :encrypted_custom_headers_iv, :binary + end +end diff --git a/db/schema_migrations/20240305201830 b/db/schema_migrations/20240305201830 new file mode 100644 index 0000000000000000000000000000000000000000..8437fff3d55b66c47e99cf0888e657679b0b33c6 --- /dev/null +++ b/db/schema_migrations/20240305201830 @@ -0,0 +1 @@ +98d913f7773e5ff2579232c0b6137b82f3b678558a1c24ab209a3e24b3e897d9 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 3d7fe11dc84a7a5bb64b9781c9f5899d32867ca8..cb8e605b64e0ed2dbb42eea25b28d03a623c6d49 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17539,6 +17539,8 @@ CREATE TABLE web_hooks ( description text, custom_webhook_template text, resource_access_token_events boolean DEFAULT false NOT NULL, + encrypted_custom_headers bytea, + encrypted_custom_headers_iv bytea, CONSTRAINT check_1e4d5cbdc5 CHECK ((char_length(name) <= 255)), CONSTRAINT check_23a96ad211 CHECK ((char_length(description) <= 2048)), CONSTRAINT check_69ef76ee0c CHECK ((char_length(custom_webhook_template) <= 4096)) diff --git a/doc/user/project/integrations/webhooks.md b/doc/user/project/integrations/webhooks.md index 9d827ad52cea2c298dd931da90cedb2c04102643..4d56b7f5844a62e8bff084ccd4a52748f1271ca4 100644 --- a/doc/user/project/integrations/webhooks.md +++ b/doc/user/project/integrations/webhooks.md @@ -106,6 +106,27 @@ You must define the following variables: Variable names can contain only lowercase letters (`a-z`), numbers (`0-9`), or underscores (`_`). You can define URL variables directly using the REST API. +## Custom headers + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/146702) in GitLab 16.11 [with a flag](../../../administration/feature_flags.md) named `custom_webhook_headers`. Disabled by default. + +FLAG: +On self-managed GitLab, by default this feature is not available. To make it available, an administrator can +[enable the feature flag](../../../administration/feature_flags.md) named `custom_webhook_headers`. +On GitLab.com and GitLab Dedicated, this feature is not available. + +You can add up to 20 custom headers in the webhook configuration as part of the request. +You can use these custom headers for authentication to external services. + +Custom headers must not override the values of [delivery headers](#delivery-headers). +The header name must: + +- Contain only alphanumeric characters, periods, dashes, or underscores. +- Start with a letter and end with a letter or number. +- Have no consecutive periods, dashes, or underscores. + +Custom headers appear in [recent deliveries](#recently-triggered-webhook-payloads-in-gitlab-settings) with masked values. + ## Custom webhook template > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142738) in GitLab 16.10 [with a flag](../../../administration/feature_flags.md) named `custom_webhook_template`. Enabled by default. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 119a15b4b0a5dc2b5f2561c6a20bdb9cbe7add6f..fdafbfee18411d1d5757269032a2ef738330513f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -56241,6 +56241,9 @@ msgstr "" msgid "Webhooks|A wiki page is created or updated." msgstr "" +msgid "Webhooks|Add custom header" +msgstr "" + msgid "Webhooks|An access token is going to expire in the next 7 days. %{help_link}?" msgstr "" @@ -56265,6 +56268,9 @@ msgstr "" msgid "Webhooks|Confidential issues events" msgstr "" +msgid "Webhooks|Custom headers" +msgstr "" + msgid "Webhooks|Custom webhook template (optional)" msgstr "" @@ -56295,6 +56301,12 @@ msgstr "" msgid "Webhooks|Go to webhooks" msgstr "" +msgid "Webhooks|Header name" +msgstr "" + +msgid "Webhooks|Header value" +msgstr "" + msgid "Webhooks|How it looks in the UI" msgstr "" @@ -56322,6 +56334,12 @@ msgstr "" msgid "Webhooks|Name (optional)" msgstr "" +msgid "Webhooks|No custom headers configured." +msgstr "" + +msgid "Webhooks|Only alphanumeric characters, periods, dashes, and underscores allowed. Must start with a letter and end with a letter or number. Cannot have consecutive periods, dashes, or underscores." +msgstr "" + msgid "Webhooks|Pipeline events" msgstr "" @@ -56406,6 +56424,9 @@ msgstr "" msgid "Webhooks|Wildcards such as %{WILDCARD_CODE_STABLE} or %{WILDCARD_CODE_PRODUCTION} are supported." msgstr "" +msgid "Webhooks|You've reached the maximum number of custom headers." +msgstr "" + msgid "Website" msgstr "" diff --git a/spec/controllers/projects/hooks_controller_spec.rb b/spec/controllers/projects/hooks_controller_spec.rb index 0d1a452350719fe6f74b73e057f48083e973a03f..af690c0d3a887e7e4856d41ad45d1f801d8315ad 100644 --- a/spec/controllers/projects/hooks_controller_spec.rb +++ b/spec/controllers/projects/hooks_controller_spec.rb @@ -66,6 +66,48 @@ 'c' => 'new' ) end + + it 'adds, updates and deletes custom headers' do + hook.update!(custom_headers: { 'a' => 'bar', 'b' => 'woo' }) + + params[:hook] = { + custom_headers: [ + { key: 'a', value: 'updated' }, + { key: 'c', value: 'new' } + ] + } + + put :update, params: params + + expect(response).to have_gitlab_http_status(:found) + expect(flash[:notice]).to include('was updated') + + expect(hook.reload.custom_headers).to eq( + 'a' => 'updated', + 'c' => 'new' + ) + end + + it 'does not update custom headers with the secret mask' do + hook.update!(custom_headers: { 'a' => 'bar' }) + + params[:hook] = { + custom_headers: [ + { key: 'a', value: WebHook::SECRET_MASK }, + { key: 'c', value: 'new' } + ] + } + + put :update, params: params + + expect(response).to have_gitlab_http_status(:found) + expect(flash[:notice]).to include('was updated') + + expect(hook.reload.custom_headers).to eq( + 'a' => 'bar', + 'c' => 'new' + ) + end end describe '#edit' do diff --git a/spec/frontend/webhooks/components/form_custom_header_item_spec.js b/spec/frontend/webhooks/components/form_custom_header_item_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..90f5ba579d87c26db5bd9adff909fb161fa999ed --- /dev/null +++ b/spec/frontend/webhooks/components/form_custom_header_item_spec.js @@ -0,0 +1,104 @@ +import { nextTick } from 'vue'; +import { GlButton, GlFormInput } from '@gitlab/ui'; + +import { s__ } from '~/locale'; +import FormCustomHeaderItem from '~/webhooks/components/form_custom_header_item.vue'; +import { MASK_ITEM_VALUE_HIDDEN } from '~/webhooks/constants'; + +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; + +describe('FormCustomHeaderItem', () => { + let wrapper; + + const createComponent = ({ props } = {}) => { + wrapper = shallowMountExtended(FormCustomHeaderItem, { + propsData: { index: 0, keyState: true, valueState: true, ...props }, + }); + }; + + const findCustomHeaderItemKey = () => wrapper.findByTestId('custom-header-item-key'); + const findCustomHeaderItemValue = () => wrapper.findByTestId('custom-header-item-value'); + const findRemoveButton = () => wrapper.findComponent(GlButton); + + it('renders input for key and value', () => { + const headerKey = 'key'; + const headerValue = 'value'; + + createComponent({ props: { headerKey, headerValue } }); + + const keyInput = findCustomHeaderItemKey(); + const valueInput = findCustomHeaderItemValue(); + + expect(keyInput.attributes()).toMatchObject({ + label: s__('Webhooks|Header name'), + }); + expect(keyInput.findComponent(GlFormInput).attributes()).toMatchObject({ + name: 'hook[custom_headers][][key]', + value: headerKey, + state: 'true', + }); + + expect(valueInput.attributes()).toMatchObject({ + label: s__('Webhooks|Header value'), + }); + expect(valueInput.findComponent(GlFormInput).attributes()).toMatchObject({ + name: 'hook[custom_headers][][value]', + value: headerValue, + state: 'true', + }); + }); + + describe('when value is the secret mask', () => { + it('renders readonly key and value', () => { + createComponent({ props: { headerKey: 'key', headerValue: MASK_ITEM_VALUE_HIDDEN } }); + + expect( + findCustomHeaderItemKey().findComponent(GlFormInput).attributes('readonly'), + ).toBeDefined(); + expect( + findCustomHeaderItemValue().findComponent(GlFormInput).attributes('readonly'), + ).toBeDefined(); + }); + }); + + it('renders remove button', () => { + createComponent({ props: { headerKey: 'key', headerValue: 'value' } }); + + expect(findRemoveButton().props('icon')).toBe('remove'); + }); + + describe('when remove button is clicked', () => { + it('emits remove event', async () => { + createComponent({ props: { headerKey: 'key', headerValue: 'value' } }); + + findRemoveButton().vm.$emit('click'); + await nextTick(); + + expect(wrapper.emitted('remove')).toHaveLength(1); + }); + }); + + describe('events', () => { + const headerKey = 'key'; + const headerValue = 'value'; + const mockInput = 'input'; + + it('update:header-key on key input', async () => { + createComponent({ props: { headerKey, headerValue } }); + + findCustomHeaderItemKey().findComponent(GlFormInput).vm.$emit('input', mockInput); + await nextTick(); + + expect(wrapper.emitted('update:header-key')).toEqual([[mockInput]]); + }); + + it('update:header-value on value input', async () => { + createComponent({ props: { headerKey, headerValue } }); + + findCustomHeaderItemValue().findComponent(GlFormInput).vm.$emit('input', mockInput); + await nextTick(); + + expect(wrapper.emitted('update:header-value')).toEqual([[mockInput]]); + }); + }); +}); diff --git a/spec/frontend/webhooks/components/form_custom_headers_spec.js b/spec/frontend/webhooks/components/form_custom_headers_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..fb7b022af3b4832057377de5bf1a5ff9aeb8d526 --- /dev/null +++ b/spec/frontend/webhooks/components/form_custom_headers_spec.js @@ -0,0 +1,137 @@ +import { nextTick } from 'vue'; + +import { scrollToElement } from '~/lib/utils/common_utils'; +import FormCustomHeaders from '~/webhooks/components/form_custom_headers.vue'; +import FormCustomHeaderItem from '~/webhooks/components/form_custom_header_item.vue'; + +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; + +jest.mock('~/lib/utils/common_utils'); + +describe('FormCustomHeaders', () => { + let wrapper; + + const createEmptyCustomHeaders = () => ({ + initialCustomHeaders: [], + }); + + const createFilledCustomHeaders = () => ({ + initialCustomHeaders: [ + { key: 'key1', value: 'value1' }, + { key: 'key2', value: 'value2' }, + ], + }); + + const createComponent = ({ props } = {}) => { + wrapper = mountExtended(FormCustomHeaders, { + propsData: props, + }); + }; + + const findCustomHeadersCard = () => wrapper.findByTestId('custom-headers-card'); + const findAllCustomHeaderItems = () => wrapper.findAllComponents(FormCustomHeaderItem); + const findAddItem = () => wrapper.findByTestId('add-custom-header'); + + describe('template', () => { + it('renders custom headers card', () => { + createComponent({ props: createEmptyCustomHeaders() }); + + expect(findCustomHeadersCard().exists()).toBe(true); + }); + + describe('when add item is clicked', () => { + it('adds custom header item', async () => { + createComponent({ props: createFilledCustomHeaders() }); + + findAddItem().vm.$emit('click'); + await nextTick(); + + expect(findAllCustomHeaderItems()).toHaveLength(3); + + const lastItem = findAllCustomHeaderItems().at(-1); + expect(lastItem.props()).toMatchObject({ + headerKey: '', + headerValue: '', + }); + }); + }); + + describe('when remove item is clicked', () => { + it('removes the correct custom header item', async () => { + createComponent({ props: createFilledCustomHeaders() }); + + const firstItem = findAllCustomHeaderItems().at(0); + firstItem.vm.$emit('remove'); + await nextTick(); + + expect(findAllCustomHeaderItems()).toHaveLength(1); + + const newFirstItem = findAllCustomHeaderItems().at(0); + expect(newFirstItem.props()).toMatchObject({ + headerKey: 'key2', + headerValue: 'value2', + }); + }); + }); + + describe('when maximum headers are reached', () => { + it('does not render add item button', () => { + createComponent({ + props: { + initialCustomHeaders: Array.from({ length: 20 }, (i) => ({ + key: `key${i}`, + value: `value${i}`, + })), + }, + }); + + expect(findAddItem().exists()).toBe(false); + }); + }); + }); + + describe('validation', () => { + beforeEach(() => { + setHTMLFixture('
'); + }); + + afterEach(() => { + resetHTMLFixture(); + }); + + const findFormEl = () => document.querySelector('.js-webhook-form'); + const submitForm = (event) => findFormEl().dispatchEvent(event); + + const createFakeSubmitEvent = () => { + const event = new Event('submit'); + event.preventDefault = jest.fn(); + event.stopPropagation = jest.fn(); + return event; + }; + + it('prevents submit event when form is invalid', async () => { + createComponent({ props: { initialCustomHeaders: [{ key: 'key', value: '' }] } }); + + const fakeEvent = createFakeSubmitEvent(); + submitForm(fakeEvent); + await nextTick(); + + expect(fakeEvent.preventDefault).toHaveBeenCalled(); + expect(fakeEvent.stopPropagation).toHaveBeenCalled(); + expect(scrollToElement).toHaveBeenCalledTimes(1); + }); + + it('does not prevent submit event when form is valid', async () => { + createComponent({ props: { initialCustomHeaders: [{ key: 'key', value: 'value' }] } }); + + const fakeEvent = createFakeSubmitEvent(); + submitForm(fakeEvent); + await nextTick(); + + expect(fakeEvent.preventDefault).not.toHaveBeenCalled(); + expect(fakeEvent.stopPropagation).not.toHaveBeenCalled(); + expect(scrollToElement).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/spec/helpers/hooks_helper_spec.rb b/spec/helpers/hooks_helper_spec.rb index d8fa64e099af614708607c56cb0ddd9c14f74efd..29abd342b88ba01edd7b442cf0971de7be397b0a 100644 --- a/spec/helpers/hooks_helper_spec.rb +++ b/spec/helpers/hooks_helper_spec.rb @@ -15,7 +15,8 @@ it 'returns proper data' do expect(subject).to match( url: project_hook.url, - url_variables: "[]" + url_variables: "[]", + custom_headers: "[]" ) end end @@ -26,7 +27,20 @@ it 'returns proper data' do expect(subject).to match( url: project_hook.url, - url_variables: Gitlab::Json.dump([{ key: 'abc' }, { key: 'def' }]) + url_variables: Gitlab::Json.dump([{ key: 'abc' }, { key: 'def' }]), + custom_headers: "[]" + ) + end + end + + context 'when there are custom headers' do + let(:project_hook) { build_stubbed(:project_hook, project: project, custom_headers: { test: 'blub' }) } + + it 'returns proper data' do + expect(subject).to match( + url: project_hook.url, + url_variables: "[]", + custom_headers: Gitlab::Json.dump([{ key: 'test', value: WebHook::SECRET_MASK }]) ) end end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 4b5ab3277db5544d38f5450ba44aaa5717dcf179..761cd3a81752446cd2efb95eb13cca4df5fd8ead 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -57,6 +57,38 @@ it { is_expected.not_to allow_value({ 'x..y' => 'foo' }).for(:url_variables) } end + describe 'custom_headers' do + it { is_expected.to allow_value({}).for(:custom_headers) } + it { is_expected.to allow_value({ 'foo' => 'bar' }).for(:custom_headers) } + it { is_expected.to allow_value({ 'FOO' => 'bar' }).for(:custom_headers) } + it { is_expected.to allow_value({ 'MY_TOKEN' => 'bar' }).for(:custom_headers) } + it { is_expected.to allow_value({ 'foo2' => 'bar' }).for(:custom_headers) } + it { is_expected.to allow_value({ 'x' => 'y' }).for(:custom_headers) } + it { is_expected.to allow_value({ 'x' => ('a' * 2048) }).for(:custom_headers) } + it { is_expected.to allow_value({ 'foo' => 'bar', 'bar' => 'baz' }).for(:custom_headers) } + it { is_expected.to allow_value((1..20).to_h { ["k#{_1}", 'value'] }).for(:custom_headers) } + it { is_expected.to allow_value({ 'MY-TOKEN' => 'bar' }).for(:custom_headers) } + it { is_expected.to allow_value({ 'my_secr3t-token' => 'bar' }).for(:custom_headers) } + it { is_expected.to allow_value({ 'x-y-z' => 'bar' }).for(:custom_headers) } + it { is_expected.to allow_value({ 'x_y_z' => 'bar' }).for(:custom_headers) } + it { is_expected.to allow_value({ 'f.o.o' => 'bar' }).for(:custom_headers) } + + it { is_expected.not_to allow_value([]).for(:custom_headers) } + it { is_expected.not_to allow_value({ 'foo' => 1 }).for(:custom_headers) } + it { is_expected.not_to allow_value({ 'bar' => :baz }).for(:custom_headers) } + it { is_expected.not_to allow_value({ 'bar' => nil }).for(:custom_headers) } + it { is_expected.not_to allow_value({ 'foo' => '' }).for(:custom_headers) } + it { is_expected.not_to allow_value({ 'foo' => ('a' * 2049) }).for(:custom_headers) } + it { is_expected.not_to allow_value({ 'has spaces' => 'foo' }).for(:custom_headers) } + it { is_expected.not_to allow_value({ '' => 'foo' }).for(:custom_headers) } + it { is_expected.not_to allow_value({ '1foo' => 'foo' }).for(:custom_headers) } + it { is_expected.not_to allow_value((1..21).to_h { ["k#{_1}", 'value'] }).for(:custom_headers) } + it { is_expected.not_to allow_value({ 'MY--TOKEN' => 'foo' }).for(:custom_headers) } + it { is_expected.not_to allow_value({ 'MY__SECRET' => 'foo' }).for(:custom_headers) } + it { is_expected.not_to allow_value({ 'x-_y' => 'foo' }).for(:custom_headers) } + it { is_expected.not_to allow_value({ 'x..y' => 'foo' }).for(:custom_headers) } + end + describe 'url' do it { is_expected.to allow_value('http://example.com').for(:url) } it { is_expected.to allow_value('https://example.com').for(:url) } @@ -287,6 +319,40 @@ end end + describe 'before_validation :reset_custom_headers' do + subject(:hook) { build_stubbed(:project_hook, :url_variables, project: project, url: 'http://example.com/{abc}', custom_headers: { test: 'blub' }) } + + it 'resets custom headers if url changed' do + hook.url = 'http://example.com/new-hook' + + expect(hook).to be_valid + expect(hook.custom_headers).to eq({}) + end + + it 'resets custom headers if url and url variables changed' do + hook.url = 'http://example.com/{something}' + hook.url_variables = { 'something' => 'testing-around' } + + expect(hook).to be_valid + expect(hook.custom_headers).to eq({}) + end + + it 'does not reset custom headers if url stayed the same' do + hook.url = 'http://example.com/{abc}' + + expect(hook).to be_valid + expect(hook.custom_headers).to eq({ test: 'blub' }) + end + + it 'does not reset custom headers if url and url variables changed and evaluate to the same url' do + hook.url = 'http://example.com/{def}' + hook.url_variables = { 'def' => 'supers3cret' } + + expect(hook).to be_valid + expect(hook.custom_headers).to eq({ test: 'blub' }) + end + end + it "only consider these branch filter strategies are valid" do expected_valid_types = %w[all_branches regex wildcard] expect(described_class.branch_filter_strategies.keys).to contain_exactly(*expected_valid_types) @@ -296,7 +362,7 @@ describe 'encrypted attributes' do subject { described_class.attr_encrypted_attributes.keys } - it { is_expected.to contain_exactly(:token, :url, :url_variables) } + it { is_expected.to contain_exactly(:token, :url, :url_variables, :custom_headers) } end describe 'execute' do diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index c3230dcd64e62b96e61d4ea1fb27689806d8580c..266b6b8f472d85bbd5f21902bb06644c41ac38c0 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -451,6 +451,48 @@ end end + context 'when custom_headers are set' do + let(:custom_headers) { { testing: 'blub', 'more-testing': 'whoops' } } + + before do + stub_full_request(project_hook.url, method: :post) + project_hook.custom_headers = custom_headers + end + + it 'sends request with custom headers' do + service_instance.execute + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(headers: custom_headers.merge(headers)) + end + + context 'when overriding predefined headers' do + let(:custom_headers) do + { Gitlab::WebHooks::RecursionDetection::UUID::HEADER => 'some overriden value' } + end + + it 'does not take user-provided value' do + service_instance.execute + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(headers: Gitlab::WebHooks::RecursionDetection.header(project_hook)) + end + end + + context 'when custom_webhook_headers feature flag is disabled' do + before do + stub_feature_flags(custom_webhook_headers: false) + end + + it 'sends request without custom headers' do + service_instance.execute + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(headers: headers) + end + end + end + it 'handles 200 status code' do stub_full_request(project_hook.url, method: :post).to_return(status: 200, body: 'Success')