From a458479636767ad21136c071f038f1b5321ea991 Mon Sep 17 00:00:00 2001 From: Dimitris <104737204+dimitris-kavvathas-sonarsource@users.noreply.github.com> Date: Thu, 15 Jun 2023 15:49:19 +0200 Subject: [PATCH] SONAR-18835 Make webhook secret private (#8479) Co-authored-by: Ambroise C --- server/sonar-web/src/main/js/api/webhooks.ts | 25 ++- .../main/js/apps/webhooks/components/App.tsx | 17 +- .../webhooks/components/CreateWebhookForm.tsx | 28 ++-- .../webhooks/components/DeleteWebhookForm.tsx | 6 +- .../webhooks/components/DeliveriesForm.tsx | 6 +- .../components/LatestDeliveryForm.tsx | 6 +- .../components/UpdateWebhookSecretField.tsx | 99 ++++++++++++ .../webhooks/components/WebhookActions.tsx | 8 +- .../apps/webhooks/components/WebhookItem.tsx | 10 +- .../components/WebhookItemLatestDelivery.tsx | 4 +- .../apps/webhooks/components/WebhooksList.tsx | 8 +- .../components/__tests__/App-test.tsx | 67 ++++++-- .../__tests__/CreateWebhookForm-it.tsx | 145 ++++++++++++++++++ .../__tests__/CreateWebhookForm-test.tsx | 43 ------ .../__tests__/DeliveriesForm-test.tsx | 2 +- .../__tests__/LatestDeliveryForm-test.tsx | 2 +- .../__tests__/WebhookActions-test.tsx | 1 + .../components/__tests__/WebhookItem-test.tsx | 21 ++- .../WebhookItemLatestDelivery-test.tsx | 1 + .../__tests__/WebhooksList-test.tsx | 4 +- .../__tests__/__snapshots__/App-test.tsx.snap | 2 + .../CreateWebhookForm-test.tsx.snap | 61 -------- .../__snapshots__/WebhookItem-test.tsx.snap | 46 +++++- .../__snapshots__/WebhooksList-test.tsx.snap | 8 +- server/sonar-web/src/main/js/types/webhook.ts | 17 +- .../server/webhook/ws/CreateActionIT.java | 27 ++-- .../sonar/server/webhook/ws/ListActionIT.java | 57 ++++--- .../server/webhook/ws/UpdateActionIT.java | 24 ++- .../sonar/server/webhook/ws/CreateAction.java | 10 +- .../sonar/server/webhook/ws/ListAction.java | 7 +- .../sonar/server/webhook/ws/UpdateAction.java | 32 +++- .../webhook/ws/example-webhook-create.json | 2 +- .../webhook/ws/example-webhooks-list.json | 5 +- .../resources/org/sonar/l10n/core.properties | 7 +- sonar-ws/src/main/protobuf/ws-webhooks.proto | 8 +- 35 files changed, 580 insertions(+), 236 deletions(-) create mode 100644 server/sonar-web/src/main/js/apps/webhooks/components/UpdateWebhookSecretField.tsx create mode 100644 server/sonar-web/src/main/js/apps/webhooks/components/__tests__/CreateWebhookForm-it.tsx delete mode 100644 server/sonar-web/src/main/js/apps/webhooks/components/__tests__/CreateWebhookForm-test.tsx delete mode 100644 server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/CreateWebhookForm-test.tsx.snap diff --git a/server/sonar-web/src/main/js/api/webhooks.ts b/server/sonar-web/src/main/js/api/webhooks.ts index aebd5bf6a71..b2965b03faf 100644 --- a/server/sonar-web/src/main/js/api/webhooks.ts +++ b/server/sonar-web/src/main/js/api/webhooks.ts @@ -20,14 +20,14 @@ import { throwGlobalError } from '../helpers/error'; import { getJSON, post, postJSON } from '../helpers/request'; import { Paging } from '../types/types'; -import { Webhook, WebhookDelivery } from '../types/webhook'; +import { + WebhookCreatePayload, + WebhookDelivery, + WebhookResponse, + WebhookUpdatePayload, +} from '../types/webhook'; -export function createWebhook(data: { - name: string; - project?: string; - secret?: string; - url: string; -}): Promise<{ webhook: Webhook }> { +export function createWebhook(data: WebhookCreatePayload): Promise<{ webhook: WebhookResponse }> { return postJSON('/api/webhooks/create', data).catch(throwGlobalError); } @@ -35,16 +35,13 @@ export function deleteWebhook(data: { webhook: string }): Promise { +export function searchWebhooks(data: { + project?: string; +}): Promise<{ webhooks: WebhookResponse[] }> { return getJSON('/api/webhooks/list', data).catch(throwGlobalError); } -export function updateWebhook(data: { - webhook: string; - name: string; - secret?: string; - url: string; -}): Promise { +export function updateWebhook(data: WebhookUpdatePayload): Promise { return post('/api/webhooks/update', data).catch(throwGlobalError); } diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/App.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/App.tsx index 6e88a09c912..3c02424500c 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/App.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/App.tsx @@ -24,7 +24,7 @@ import withComponentContext from '../../../app/components/componentContext/withC import Suggestions from '../../../components/embed-docs-modal/Suggestions'; import { translate } from '../../../helpers/l10n'; import { Component } from '../../../types/types'; -import { Webhook } from '../../../types/webhook'; +import { WebhookResponse } from '../../../types/webhook'; import PageActions from './PageActions'; import PageHeader from './PageHeader'; import WebhooksList from './WebhooksList'; @@ -36,7 +36,7 @@ interface Props { interface State { loading: boolean; - webhooks: Webhook[]; + webhooks: WebhookResponse[]; } export class App extends React.PureComponent { @@ -99,19 +99,24 @@ export class App extends React.PureComponent { }; handleUpdate = (data: { webhook: string; name: string; secret?: string; url: string }) => { - const udpateData = { + const updateData = { webhook: data.webhook, name: data.name, url: data.url, - ...(data.secret && { secret: data.secret }), + secret: data.secret, }; - return updateWebhook(udpateData).then(() => { + return updateWebhook(updateData).then(() => { if (this.mounted) { this.setState(({ webhooks }) => ({ webhooks: webhooks.map((webhook) => webhook.key === data.webhook - ? { ...webhook, name: data.name, secret: data.secret, url: data.url } + ? { + ...webhook, + name: data.name, + hasSecret: data.secret === undefined ? webhook.hasSecret : Boolean(data.secret), + url: data.url, + } : webhook ), })); diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/CreateWebhookForm.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/CreateWebhookForm.tsx index 1e2fe89da74..a2d8d0938b1 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/CreateWebhookForm.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/CreateWebhookForm.tsx @@ -24,18 +24,13 @@ import ValidationModal from '../../../components/controls/ValidationModal'; import MandatoryFieldMarker from '../../../components/ui/MandatoryFieldMarker'; import MandatoryFieldsExplanation from '../../../components/ui/MandatoryFieldsExplanation'; import { translate } from '../../../helpers/l10n'; -import { Webhook } from '../../../types/webhook'; +import { WebhookBasePayload, WebhookResponse } from '../../../types/webhook'; +import UpdateWebhookSecretField from './UpdateWebhookSecretField'; interface Props { onClose: () => void; - onDone: (data: Values) => Promise; - webhook?: Webhook; -} - -interface Values { - name: string; - secret: string; - url: string; + onDone: (data: WebhookBasePayload) => Promise; + webhook?: WebhookResponse; } export default class CreateWebhookForm extends React.PureComponent { @@ -45,7 +40,7 @@ export default class CreateWebhookForm extends React.PureComponent { this.props.onClose(); }; - handleValidate = (data: Values) => { + handleValidate = (data: WebhookBasePayload) => { const { name, secret, url } = data; const errors: { name?: string; secret?: string; url?: string } = {}; if (!name.trim()) { @@ -74,9 +69,9 @@ export default class CreateWebhookForm extends React.PureComponent { confirmButtonText={confirmButtonText} header={modalHeader} initialValues={{ - name: (webhook && webhook.name) || '', - secret: (webhook && webhook.secret) || '', - url: (webhook && webhook.url) || '', + name: webhook?.name ?? '', + url: webhook?.url ?? '', + secret: isUpdate ? undefined : '', }} onClose={this.props.onClose} onSubmit={this.props.onDone} @@ -125,12 +120,15 @@ export default class CreateWebhookForm extends React.PureComponent { type="text" value={values.url} /> - {translate('webhooks.secret')}} name="secret" onBlur={handleBlur} diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/DeleteWebhookForm.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/DeleteWebhookForm.tsx index 6ec97f173ce..238ec10c8b9 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/DeleteWebhookForm.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/DeleteWebhookForm.tsx @@ -18,16 +18,16 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ import * as React from 'react'; -import { ResetButtonLink, SubmitButton } from '../../../components/controls/buttons'; import SimpleModal from '../../../components/controls/SimpleModal'; +import { ResetButtonLink, SubmitButton } from '../../../components/controls/buttons'; import DeferredSpinner from '../../../components/ui/DeferredSpinner'; import { translate, translateWithParameters } from '../../../helpers/l10n'; -import { Webhook } from '../../../types/webhook'; +import { WebhookResponse } from '../../../types/webhook'; interface Props { onClose: () => void; onSubmit: () => Promise; - webhook: Webhook; + webhook: WebhookResponse; } export default function DeleteWebhookForm({ onClose, onSubmit, webhook }: Props) { diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/DeliveriesForm.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/DeliveriesForm.tsx index 9dfad0d5b71..85e70128724 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/DeliveriesForm.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/DeliveriesForm.tsx @@ -19,18 +19,18 @@ */ import * as React from 'react'; import { searchDeliveries } from '../../../api/webhooks'; -import { ResetButtonLink } from '../../../components/controls/buttons'; import ListFooter from '../../../components/controls/ListFooter'; import Modal from '../../../components/controls/Modal'; +import { ResetButtonLink } from '../../../components/controls/buttons'; import DeferredSpinner from '../../../components/ui/DeferredSpinner'; import { translate, translateWithParameters } from '../../../helpers/l10n'; import { Paging } from '../../../types/types'; -import { Webhook, WebhookDelivery } from '../../../types/webhook'; +import { WebhookDelivery, WebhookResponse } from '../../../types/webhook'; import DeliveryAccordion from './DeliveryAccordion'; interface Props { onClose: () => void; - webhook: Webhook; + webhook: WebhookResponse; } interface State { diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/LatestDeliveryForm.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/LatestDeliveryForm.tsx index d4dba71fc42..ceedc15589c 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/LatestDeliveryForm.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/LatestDeliveryForm.tsx @@ -19,16 +19,16 @@ */ import * as React from 'react'; import { getDelivery } from '../../../api/webhooks'; -import { ResetButtonLink } from '../../../components/controls/buttons'; import Modal from '../../../components/controls/Modal'; +import { ResetButtonLink } from '../../../components/controls/buttons'; import { translate, translateWithParameters } from '../../../helpers/l10n'; -import { Webhook, WebhookDelivery } from '../../../types/webhook'; +import { WebhookDelivery, WebhookResponse } from '../../../types/webhook'; import DeliveryItem from './DeliveryItem'; interface Props { delivery: WebhookDelivery; onClose: () => void; - webhook: Webhook; + webhook: WebhookResponse; } interface State { diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/UpdateWebhookSecretField.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/UpdateWebhookSecretField.tsx new file mode 100644 index 00000000000..1d4b8def53f --- /dev/null +++ b/server/sonar-web/src/main/js/apps/webhooks/components/UpdateWebhookSecretField.tsx @@ -0,0 +1,99 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +import { useField } from 'formik'; +import * as React from 'react'; +import InputValidationField from '../../../components/controls/InputValidationField'; +import ModalValidationField from '../../../components/controls/ModalValidationField'; +import { ButtonLink } from '../../../components/controls/buttons'; +import { translate } from '../../../helpers/l10n'; + +interface Props { + description?: string; + dirty: boolean; + disabled: boolean; + error: string | undefined; + id?: string; + isUpdateForm: boolean; + label?: React.ReactNode; + name: string; + onBlur: (event: React.FocusEvent) => void; + onChange: (event: React.ChangeEvent) => void; + touched: boolean | undefined; + type?: string; + value?: string; +} + +export default function UpdateWebhookSecretField({ + isUpdateForm, + description, + dirty, + disabled, + error, + id, + label, + name, + onBlur, + onChange, + touched, + type, + value, +}: Props) { + const [isSecretInputDisplayed, setIsSecretInputDisplayed] = React.useState(false); + const [, , { setValue: setSecretValue }] = useField('secret'); + + const showSecretInput = () => { + setSecretValue(''); + setIsSecretInputDisplayed(true); + }; + + return !isUpdateForm || isSecretInputDisplayed ? ( + + ) : ( + + {() => ( +
+ {translate('webhooks.secret.field_mask.description')} + + {translate('webhooks.secret.field_mask.link')} + +
+ )} + + ); +} diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/WebhookActions.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/WebhookActions.tsx index b463b86e9b0..d8338c3cb37 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/WebhookActions.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/WebhookActions.tsx @@ -23,15 +23,15 @@ import ActionsDropdown, { ActionsDropdownItem, } from '../../../components/controls/ActionsDropdown'; import { translate, translateWithParameters } from '../../../helpers/l10n'; -import { Webhook } from '../../../types/webhook'; +import { WebhookResponse, WebhookUpdatePayload } from '../../../types/webhook'; import CreateWebhookForm from './CreateWebhookForm'; import DeleteWebhookForm from './DeleteWebhookForm'; import DeliveriesForm from './DeliveriesForm'; interface Props { onDelete: (webhook: string) => Promise; - onUpdate: (data: { webhook: string; name: string; url: string }) => Promise; - webhook: Webhook; + onUpdate: (data: WebhookUpdatePayload) => Promise; + webhook: WebhookResponse; } interface State { @@ -74,7 +74,7 @@ export default class WebhookActions extends React.PureComponent { this.setState({ deliveries: false }); }; - handleUpdate = (data: { name: string; url: string }) => { + handleUpdate = (data: { name: string; secret?: string; url: string }) => { return this.props.onUpdate({ ...data, webhook: this.props.webhook.key }); }; diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/WebhookItem.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/WebhookItem.tsx index 51ef3dc2236..1e69c832adc 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/WebhookItem.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/WebhookItem.tsx @@ -19,14 +19,14 @@ */ import * as React from 'react'; import { translate } from '../../../helpers/l10n'; -import { Webhook } from '../../../types/webhook'; +import { WebhookResponse, WebhookUpdatePayload } from '../../../types/webhook'; import WebhookActions from './WebhookActions'; import WebhookItemLatestDelivery from './WebhookItemLatestDelivery'; interface Props { onDelete: (webhook: string) => Promise; - onUpdate: (data: { webhook: string; name: string; url: string }) => Promise; - webhook: Webhook; + onUpdate: (data: WebhookUpdatePayload) => Promise; + webhook: WebhookResponse; } export default function WebhookItem({ onDelete, onUpdate, webhook }: Props) { @@ -34,11 +34,11 @@ export default function WebhookItem({ onDelete, onUpdate, webhook }: Props) { {webhook.name} {webhook.url} - {webhook.secret ? translate('yes') : translate('no')} + {webhook.hasSecret ? translate('yes') : translate('no')} - + diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/WebhookItemLatestDelivery.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/WebhookItemLatestDelivery.tsx index d932cf8633a..c896aa43738 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/WebhookItemLatestDelivery.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/WebhookItemLatestDelivery.tsx @@ -24,11 +24,11 @@ import AlertSuccessIcon from '../../../components/icons/AlertSuccessIcon'; import BulletListIcon from '../../../components/icons/BulletListIcon'; import DateTimeFormatter from '../../../components/intl/DateTimeFormatter'; import { translate } from '../../../helpers/l10n'; -import { Webhook } from '../../../types/webhook'; +import { WebhookResponse } from '../../../types/webhook'; import LatestDeliveryForm from './LatestDeliveryForm'; interface Props { - webhook: Webhook; + webhook: WebhookResponse; } interface State { diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/WebhooksList.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/WebhooksList.tsx index 08135a70143..b60aa9008c4 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/WebhooksList.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/WebhooksList.tsx @@ -20,13 +20,13 @@ import { sortBy } from 'lodash'; import * as React from 'react'; import { translate } from '../../../helpers/l10n'; -import { Webhook } from '../../../types/webhook'; +import { WebhookResponse, WebhookUpdatePayload } from '../../../types/webhook'; import WebhookItem from './WebhookItem'; interface Props { onDelete: (webhook: string) => Promise; - onUpdate: (data: { webhook: string; name: string; url: string }) => Promise; - webhooks: Webhook[]; + onUpdate: (data: WebhookUpdatePayload) => Promise; + webhooks: WebhookResponse[]; } export default class WebhooksList extends React.PureComponent { @@ -37,7 +37,7 @@ export default class WebhooksList extends React.PureComponent { {translate('webhooks.url')} {translate('webhooks.secret_header')} {translate('webhooks.last_execution')} - + {translate('actions')} ); diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/App-test.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/App-test.tsx index 69fd13fd6d4..1aa92f4b207 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/App-test.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/App-test.tsx @@ -30,14 +30,14 @@ import { App } from '../App'; jest.mock('../../../../api/webhooks', () => ({ createWebhook: jest.fn(() => - Promise.resolve({ webhook: { key: '3', name: 'baz', url: 'http://baz' } }) + Promise.resolve({ webhook: { key: '3', name: 'baz', url: 'http://baz', hasSecret: false } }) ), deleteWebhook: jest.fn(() => Promise.resolve()), searchWebhooks: jest.fn(() => Promise.resolve({ webhooks: [ - { key: '1', name: 'foo', url: 'http://foo' }, - { key: '2', name: 'bar', url: 'http://bar' }, + { key: '1', name: 'foo', url: 'http://foo', hasSecret: false }, + { key: '2', name: 'bar', url: 'http://bar', hasSecret: false }, ], }) ), @@ -98,9 +98,9 @@ it('should correctly handle webhook creation', async () => { await new Promise(setImmediate); wrapper.update(); expect(wrapper.state('webhooks')).toEqual([ - { key: '1', name: 'foo', url: 'http://foo' }, - { key: '2', name: 'bar', url: 'http://bar' }, - { key: '3', name: 'baz', url: 'http://baz' }, + { key: '1', name: 'foo', url: 'http://foo', hasSecret: false }, + { key: '2', name: 'bar', url: 'http://bar', hasSecret: false }, + { key: '3', name: 'baz', url: 'http://baz', hasSecret: false }, ]); }); @@ -111,11 +111,13 @@ it('should correctly handle webhook deletion', async () => { await new Promise(setImmediate); wrapper.update(); - expect(wrapper.state('webhooks')).toEqual([{ key: '1', name: 'foo', url: 'http://foo' }]); + expect(wrapper.state('webhooks')).toEqual([ + { key: '1', name: 'foo', url: 'http://foo', hasSecret: false }, + ]); }); it('should correctly handle webhook update', async () => { - const newValues = { webhook: '1', name: 'Cfoo', url: 'http://cfoo' }; + const newValues = { webhook: '1', name: 'Cfoo', url: 'http://cfoo', secret: undefined }; const wrapper = shallow(); (wrapper.instance() as App).handleUpdate(newValues); expect(updateWebhook).toHaveBeenLastCalledWith(newValues); @@ -123,7 +125,52 @@ it('should correctly handle webhook update', async () => { await new Promise(setImmediate); wrapper.update(); expect(wrapper.state('webhooks')).toEqual([ - { key: '1', name: 'Cfoo', url: 'http://cfoo' }, - { key: '2', name: 'bar', url: 'http://bar' }, + { key: '1', name: 'Cfoo', url: 'http://cfoo', hasSecret: false }, + { key: '2', name: 'bar', url: 'http://bar', hasSecret: false }, + ]); +}); + +it('should correctly handle webhook secret update', async () => { + const newValuesWithSecret = { webhook: '2', name: 'bar', url: 'http://bar', secret: 'secret' }; + const newValuesWithoutSecret = { + webhook: '2', + name: 'bar', + url: 'http://bar', + secret: undefined, + }; + const newValuesWithEmptySecret = { webhook: '2', name: 'bar', url: 'http://bar', secret: '' }; + const wrapper = shallow(); + + // With secret + (wrapper.instance() as App).handleUpdate(newValuesWithSecret); + expect(updateWebhook).toHaveBeenLastCalledWith(newValuesWithSecret); + + await new Promise(setImmediate); + wrapper.update(); + expect(wrapper.state('webhooks')).toEqual([ + { key: '1', name: 'foo', url: 'http://foo', hasSecret: false }, + { key: '2', name: 'bar', url: 'http://bar', hasSecret: true }, + ]); + + // Without secret + (wrapper.instance() as App).handleUpdate(newValuesWithoutSecret); + expect(updateWebhook).toHaveBeenLastCalledWith(newValuesWithoutSecret); + + await new Promise(setImmediate); + wrapper.update(); + expect(wrapper.state('webhooks')).toEqual([ + { key: '1', name: 'foo', url: 'http://foo', hasSecret: false }, + { key: '2', name: 'bar', url: 'http://bar', hasSecret: true }, + ]); + + // With empty secret + (wrapper.instance() as App).handleUpdate(newValuesWithEmptySecret); + expect(updateWebhook).toHaveBeenLastCalledWith(newValuesWithEmptySecret); + + await new Promise(setImmediate); + wrapper.update(); + expect(wrapper.state('webhooks')).toEqual([ + { key: '1', name: 'foo', url: 'http://foo', hasSecret: false }, + { key: '2', name: 'bar', url: 'http://bar', hasSecret: false }, ]); }); diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/CreateWebhookForm-it.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/CreateWebhookForm-it.tsx new file mode 100644 index 00000000000..c56875897d3 --- /dev/null +++ b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/CreateWebhookForm-it.tsx @@ -0,0 +1,145 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +import userEvent from '@testing-library/user-event'; +import * as React from 'react'; +import { byLabelText, byRole } from 'testing-library-selector'; +import { renderComponent } from '../../../../helpers/testReactTestingUtils'; +import CreateWebhookForm from '../CreateWebhookForm'; + +const ui = { + nameInput: byRole('textbox', { name: 'webhooks.name field_required' }), + urlInput: byRole('textbox', { name: 'webhooks.url field_required' }), + secretInput: byLabelText('webhooks.secret'), + secretInputMaskButton: byRole('button', { name: 'webhooks.secret.field_mask.link' }), + createButton: byRole('button', { name: 'create' }), + updateButton: byRole('button', { name: 'update_verb' }), +}; + +describe('Webhook form', () => { + it('should correctly submit creation form', async () => { + const user = userEvent.setup(); + const webhook = { + name: 'foo', + url: 'http://bar', + secret: '', + }; + const onDone = jest.fn(); + + renderCreateWebhookForm({ onDone }); + + expect(ui.nameInput.get()).toHaveValue(''); + expect(ui.urlInput.get()).toHaveValue(''); + expect(ui.secretInput.get()).toHaveValue(''); + expect(ui.createButton.get()).toBeDisabled(); + + await user.type(ui.nameInput.get(), webhook.name); + await user.type(ui.urlInput.get(), webhook.url); + expect(ui.createButton.get()).toBeEnabled(); + + await user.click(ui.createButton.get()); + expect(onDone).toHaveBeenCalledWith(webhook); + }); + + it('should correctly submit update form', async () => { + const user = userEvent.setup(); + const webhook = { + hasSecret: false, + key: 'test-webhook-key', + name: 'foo', + url: 'http://bar', + }; + const nameExtension = 'bar'; + const url = 'http://bar'; + const onDone = jest.fn(); + + renderCreateWebhookForm({ onDone, webhook }); + + expect(ui.nameInput.get()).toHaveValue(webhook.name); + expect(ui.urlInput.get()).toHaveValue(webhook.url); + expect(ui.secretInput.query()).not.toBeInTheDocument(); + expect(ui.secretInputMaskButton.get()).toBeInTheDocument(); + expect(ui.updateButton.get()).toBeDisabled(); + + await user.type(ui.nameInput.get(), nameExtension); + await user.clear(ui.urlInput.get()); + await user.type(ui.urlInput.get(), url); + expect(ui.updateButton.get()).toBeEnabled(); + + await user.click(ui.updateButton.get()); + expect(onDone).toHaveBeenCalledWith({ + name: `${webhook.name}${nameExtension}`, + url, + secret: undefined, + }); + }); + + it('should correctly submit update form with empty secret', async () => { + const user = userEvent.setup(); + const webhook = { + hasSecret: false, + key: 'test-webhook-key', + name: 'foo', + url: 'http://bar', + }; + const onDone = jest.fn(); + + renderCreateWebhookForm({ onDone, webhook }); + + await user.click(ui.secretInputMaskButton.get()); + expect(ui.updateButton.get()).toBeEnabled(); + + await user.click(ui.updateButton.get()); + expect(onDone).toHaveBeenCalledWith({ + name: webhook.name, + url: webhook.url, + secret: '', + }); + }); + + it('should correctly submit update form with updated secret', async () => { + const user = userEvent.setup(); + const webhook = { + hasSecret: false, + key: 'test-webhook-key', + name: 'foo', + url: 'http://bar', + }; + const secret = 'test-webhook-secret'; + const onDone = jest.fn(); + + renderCreateWebhookForm({ onDone, webhook }); + + await user.click(ui.secretInputMaskButton.get()); + await user.type(ui.secretInput.get(), secret); + + await user.click(ui.updateButton.get()); + expect(onDone).toHaveBeenCalledWith({ + name: webhook.name, + url: webhook.url, + secret, + }); + }); +}); + +function renderCreateWebhookForm(props = {}) { + return renderComponent( + Promise.resolve())} {...props} /> + ); +} diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/CreateWebhookForm-test.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/CreateWebhookForm-test.tsx deleted file mode 100644 index c8ec8b5ff71..00000000000 --- a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/CreateWebhookForm-test.tsx +++ /dev/null @@ -1,43 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2023 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -import { shallow } from 'enzyme'; -import * as React from 'react'; -import CreateWebhookForm from '../CreateWebhookForm'; - -const webhookWithoutSecret = { key: '1', name: 'foo', url: 'http://foo.bar' }; -const webhookWithSecret = { key: '2', name: 'bar', secret: 'sonar', url: 'http://foo.bar' }; - -it('should render correctly when creating a new webhook', () => { - expect(getWrapper()).toMatchSnapshot(); -}); - -it('should render correctly when updating a webhook without secret', () => { - expect(getWrapper({ webhook: webhookWithoutSecret })).toMatchSnapshot(); -}); - -it('should render correctly when updating a webhook with a secret', () => { - expect(getWrapper({ webhook: webhookWithSecret })).toMatchSnapshot(); -}); - -function getWrapper(props = {}) { - return shallow( - Promise.resolve())} {...props} /> - ); -} diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/DeliveriesForm-test.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/DeliveriesForm-test.tsx index 7a04114d953..35f1d6da9aa 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/DeliveriesForm-test.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/DeliveriesForm-test.tsx @@ -50,7 +50,7 @@ jest.mock('../../../../api/webhooks', () => ({ ), })); -const webhook = { key: '1', name: 'foo', url: 'http://foo.bar' }; +const webhook = { key: '1', name: 'foo', url: 'http://foo.bar', hasSecret: false }; beforeEach(() => { (searchDeliveries as jest.Mock).mockClear(); diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/LatestDeliveryForm-test.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/LatestDeliveryForm-test.tsx index 0aac1590d78..83756ec0ee7 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/LatestDeliveryForm-test.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/LatestDeliveryForm-test.tsx @@ -38,7 +38,7 @@ const delivery = { success: true, }; -const webhook = { key: '1', name: 'foo', url: 'http://foo.bar' }; +const webhook = { key: '1', name: 'foo', url: 'http://foo.bar', hasSecret: false }; beforeEach(() => { (getDelivery as jest.Mock).mockClear(); diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhookActions-test.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhookActions-test.tsx index 6c5bb8f0616..f9e3da4f89f 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhookActions-test.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhookActions-test.tsx @@ -26,6 +26,7 @@ const webhook = { key: '1', name: 'foo', url: 'http://foo.bar', + hasSecret: false, }; const delivery = { diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhookItem-test.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhookItem-test.tsx index 1e5361f5b46..7741671cd32 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhookItem-test.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhookItem-test.tsx @@ -21,10 +21,18 @@ import { shallow } from 'enzyme'; import * as React from 'react'; import WebhookItem from '../WebhookItem'; -const webhook = { +const webhookWithoutSecret = { key: '1', name: 'my webhook', url: 'http://webhook.target', + hasSecret: false, +}; + +const webhookWithSecret = { + key: '1', + name: 'my webhook', + url: 'http://webhook.target', + hasSecret: true, }; it('should render correctly', () => { @@ -33,7 +41,16 @@ it('should render correctly', () => { Promise.resolve())} onUpdate={jest.fn(() => Promise.resolve())} - webhook={webhook} + webhook={webhookWithoutSecret} + /> + ) + ).toMatchSnapshot(); + expect( + shallow( + Promise.resolve())} + onUpdate={jest.fn(() => Promise.resolve())} + webhook={webhookWithSecret} /> ) ).toMatchSnapshot(); diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhookItemLatestDelivery-test.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhookItemLatestDelivery-test.tsx index 312bb572b7d..ea960a1e764 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhookItemLatestDelivery-test.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhookItemLatestDelivery-test.tsx @@ -34,6 +34,7 @@ const webhook = { key: '1', name: 'my webhook', url: 'http://webhook.target', + hasSecret: false, latestDelivery, }; diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhooksList-test.tsx b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhooksList-test.tsx index 15210f6c747..2f3bcace219 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhooksList-test.tsx +++ b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/WebhooksList-test.tsx @@ -22,8 +22,8 @@ import * as React from 'react'; import WebhooksList from '../WebhooksList'; const webhooks = [ - { key: '1', name: 'my webhook', url: 'http://webhook.target' }, - { key: '2', name: 'jenkins webhook', url: 'http://jenkins.target' }, + { key: '1', name: 'my webhook', url: 'http://webhook.target', hasSecret: false }, + { key: '2', name: 'jenkins webhook', url: 'http://jenkins.target', hasSecret: false }, ]; it('should correctly render empty webhook list', () => { diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/App-test.tsx.snap b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/App-test.tsx.snap index 7a0ea7fcc9f..ba0b2bb2a08 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/App-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/App-test.tsx.snap @@ -59,11 +59,13 @@ exports[`should fetch webhooks and display them 1`] = ` webhooks={ [ { + "hasSecret": false, "key": "1", "name": "foo", "url": "http://foo", }, { + "hasSecret": false, "key": "2", "name": "bar", "url": "http://bar", diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/CreateWebhookForm-test.tsx.snap b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/CreateWebhookForm-test.tsx.snap deleted file mode 100644 index e66235b6d61..00000000000 --- a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/CreateWebhookForm-test.tsx.snap +++ /dev/null @@ -1,61 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`should render correctly when creating a new webhook 1`] = ` - - - -`; - -exports[`should render correctly when updating a webhook with a secret 1`] = ` - - - -`; - -exports[`should render correctly when updating a webhook without secret 1`] = ` - - - -`; diff --git a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/WebhookItem-test.tsx.snap b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/WebhookItem-test.tsx.snap index cc748f5df22..6e31377430a 100644 --- a/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/WebhookItem-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/WebhookItem-test.tsx.snap @@ -15,6 +15,7 @@ exports[`should render correctly 1`] = ` + + +`; + +exports[`should render correctly 2`] = ` + + + my webhook + + + http://webhook.target + + + yes + + + + + + webhooks.last_execution - + + actions + @@ -34,6 +38,7 @@ exports[`should correctly render the webhooks 1`] = ` onUpdate={[MockFunction]} webhook={ { + "hasSecret": false, "key": "2", "name": "jenkins webhook", "url": "http://jenkins.target", @@ -46,6 +51,7 @@ exports[`should correctly render the webhooks 1`] = ` onUpdate={[MockFunction]} webhook={ { + "hasSecret": false, "key": "1", "name": "my webhook", "url": "http://webhook.target", diff --git a/server/sonar-web/src/main/js/types/webhook.ts b/server/sonar-web/src/main/js/types/webhook.ts index 296eae73747..0ddc6003ad9 100644 --- a/server/sonar-web/src/main/js/types/webhook.ts +++ b/server/sonar-web/src/main/js/types/webhook.ts @@ -17,14 +17,29 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -export interface Webhook { + +export interface WebhookResponse { + hasSecret: boolean; key: string; latestDelivery?: WebhookDelivery; + name: string; + url: string; +} + +export interface WebhookBasePayload { name: string; secret?: string; url: string; } +export interface WebhookCreatePayload extends WebhookBasePayload { + project?: string; +} + +export interface WebhookUpdatePayload extends WebhookBasePayload { + webhook: string; +} + export interface WebhookDelivery { at: string; durationMs: number; diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/webhook/ws/CreateActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/webhook/ws/CreateActionIT.java index 9e0b8e2ed04..10bca756e84 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/webhook/ws/CreateActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/webhook/ws/CreateActionIT.java @@ -111,7 +111,7 @@ public class CreateActionIT { assertThat(response.getWebhook().getKey()).isNotNull(); assertThat(response.getWebhook().getName()).isEqualTo(NAME_WEBHOOK_EXAMPLE_001); assertThat(response.getWebhook().getUrl()).isEqualTo(URL_WEBHOOK_EXAMPLE_001); - assertThat(response.getWebhook().getSecret()).isEqualTo("a_secret"); + assertThat(response.getWebhook().getHasSecret()).isTrue(); } @Test @@ -128,7 +128,16 @@ public class CreateActionIT { assertThat(response.getWebhook().getKey()).isNotNull(); assertThat(response.getWebhook().getName()).isEqualTo(NAME_WEBHOOK_EXAMPLE_001); assertThat(response.getWebhook().getUrl()).isEqualTo(URL_WEBHOOK_EXAMPLE_001); - assertThat(response.getWebhook().getSecret()).isEqualTo("a_secret"); + assertThat(response.getWebhook().getHasSecret()).isTrue(); + + assertThat(webhookDbTester.selectWebhook(response.getWebhook().getKey())) + .isPresent() + .hasValueSatisfying(reloaded -> { + assertThat(reloaded.getName()).isEqualTo(NAME_WEBHOOK_EXAMPLE_001); + assertThat(reloaded.getUrl()).isEqualTo(URL_WEBHOOK_EXAMPLE_001); + assertThat(reloaded.getProjectUuid()).isNull(); + assertThat(reloaded.getSecret()).isEqualTo("a_secret"); + }); } @Test @@ -144,7 +153,7 @@ public class CreateActionIT { assertThat(response.getWebhook().getKey()).isNotNull(); assertThat(response.getWebhook().getName()).isEqualTo(NAME_WEBHOOK_EXAMPLE_001); assertThat(response.getWebhook().getUrl()).isEqualTo(URL_WEBHOOK_EXAMPLE_001); - assertThat(response.getWebhook().hasSecret()).isFalse(); + assertThat(response.getWebhook().getHasSecret()).isFalse(); } @Test @@ -163,20 +172,20 @@ public class CreateActionIT { assertThat(response.getWebhook().getKey()).isNotNull(); assertThat(response.getWebhook().getName()).isEqualTo(NAME_WEBHOOK_EXAMPLE_001); assertThat(response.getWebhook().getUrl()).isEqualTo(URL_WEBHOOK_EXAMPLE_001); - assertThat(response.getWebhook().hasSecret()).isFalse(); + assertThat(response.getWebhook().getHasSecret()).isFalse(); } @Test public void fail_if_project_does_not_exist() { userSession.logIn(); TestRequest request = wsActionTester.newRequest() - .setParam(PROJECT_KEY_PARAM, "inexistent-project-uuid") + .setParam(PROJECT_KEY_PARAM, "nonexistent-project-uuid") .setParam(NAME_PARAM, NAME_WEBHOOK_EXAMPLE_001) .setParam(URL_PARAM, URL_WEBHOOK_EXAMPLE_001); assertThatThrownBy(request::execute) .isInstanceOf(NotFoundException.class) - .hasMessage("Project 'inexistent-project-uuid' not found"); + .hasMessage("Project 'nonexistent-project-uuid' not found"); } @Test @@ -285,11 +294,7 @@ public class CreateActionIT { } private static String generateStringWithLength(int length) { - StringBuilder sb = new StringBuilder(length); - for (int i = 0; i < length; i++) { - sb.append("x"); - } - return sb.toString(); + return "x".repeat(Math.max(0, length)); } } diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/webhook/ws/ListActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/webhook/ws/ListActionIT.java index 346da9c3883..1d00b408567 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/webhook/ws/ListActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/webhook/ws/ListActionIT.java @@ -43,7 +43,6 @@ import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.WsActionTester; -import org.sonarqube.ws.Webhooks; import org.sonarqube.ws.Webhooks.ListResponse; import static org.assertj.core.api.Assertions.assertThat; @@ -55,6 +54,8 @@ import static org.sonar.db.webhook.WebhookDeliveryTesting.newDto; import static org.sonar.db.webhook.WebhookTesting.newGlobalWebhook; import static org.sonar.server.tester.UserSessionRule.standalone; import static org.sonar.server.webhook.ws.WebhooksWsParameters.PROJECT_KEY_PARAM; +import static org.sonarqube.ws.Webhooks.LatestDelivery; +import static org.sonarqube.ws.Webhooks.ListResponseElement; public class ListActionIT { @@ -92,7 +93,7 @@ public class ListActionIT { .extracting(Param::key, Param::isRequired) .containsExactlyInAnyOrder( tuple("project", false)); - assertThat(action.changelog()).hasSize(1); + assertThat(action.changelog()).hasSize(2); } @Test @@ -109,18 +110,18 @@ public class ListActionIT { ListResponse response = wsActionTester.newRequest().executeProtobuf(ListResponse.class); - List elements = response.getWebhooksList(); + List elements = response.getWebhooksList(); assertThat(elements).hasSize(2); - assertThat(elements.get(0)).extracting(Webhooks.ListResponseElement::getKey).isEqualTo(webhook1.getUuid()); - assertThat(elements.get(0)).extracting(Webhooks.ListResponseElement::getName).isEqualTo("aaa"); + assertThat(elements.get(0)).extracting(ListResponseElement::getKey).isEqualTo(webhook1.getUuid()); + assertThat(elements.get(0)).extracting(ListResponseElement::getName).isEqualTo("aaa"); assertThat(elements.get(0).getLatestDelivery()).isNotNull(); - assertThat(elements.get(0).getLatestDelivery()).extracting(Webhooks.LatestDelivery::getId).isEqualTo("WH1-DELIVERY-2-UUID"); + assertThat(elements.get(0).getLatestDelivery()).extracting(LatestDelivery::getId).isEqualTo("WH1-DELIVERY-2-UUID"); - assertThat(elements.get(1)).extracting(Webhooks.ListResponseElement::getKey).isEqualTo(webhook2.getUuid()); - assertThat(elements.get(1)).extracting(Webhooks.ListResponseElement::getName).isEqualTo("bbb"); + assertThat(elements.get(1)).extracting(ListResponseElement::getKey).isEqualTo(webhook2.getUuid()); + assertThat(elements.get(1)).extracting(ListResponseElement::getName).isEqualTo("bbb"); assertThat(elements.get(1).getLatestDelivery()).isNotNull(); - assertThat(elements.get(1).getLatestDelivery()).extracting(Webhooks.LatestDelivery::getId).isEqualTo("WH2-DELIVERY-2-UUID"); + assertThat(elements.get(1).getLatestDelivery()).extracting(LatestDelivery::getId).isEqualTo("WH2-DELIVERY-2-UUID"); } @Test @@ -132,15 +133,15 @@ public class ListActionIT { ListResponse response = wsActionTester.newRequest().executeProtobuf(ListResponse.class); - List elements = response.getWebhooksList(); + List elements = response.getWebhooksList(); assertThat(elements).hasSize(2); - assertThat(elements.get(0)).extracting(Webhooks.ListResponseElement::getKey).isEqualTo(webhook1.getUuid()); - assertThat(elements.get(0)).extracting(Webhooks.ListResponseElement::getName).isEqualTo("aaa"); + assertThat(elements.get(0)).extracting(ListResponseElement::getKey).isEqualTo(webhook1.getUuid()); + assertThat(elements.get(0)).extracting(ListResponseElement::getName).isEqualTo("aaa"); assertThat(elements.get(0).hasLatestDelivery()).isFalse(); - assertThat(elements.get(1)).extracting(Webhooks.ListResponseElement::getKey).isEqualTo(webhook2.getUuid()); - assertThat(elements.get(1)).extracting(Webhooks.ListResponseElement::getName).isEqualTo("bbb"); + assertThat(elements.get(1)).extracting(ListResponseElement::getKey).isEqualTo(webhook2.getUuid()); + assertThat(elements.get(1)).extracting(ListResponseElement::getName).isEqualTo("bbb"); assertThat(elements.get(1).hasLatestDelivery()).isFalse(); } @@ -157,17 +158,17 @@ public class ListActionIT { ListResponse response = wsActionTester.newRequest().executeProtobuf(ListResponse.class); - List elements = response.getWebhooksList(); + List elements = response.getWebhooksList(); assertThat(elements) .hasSize(2) - .extracting(Webhooks.ListResponseElement::getUrl) + .extracting(ListResponseElement::getUrl) .containsOnly(expectedUrl); } @Test public void list_global_webhooks() { WebhookDto dto1 = webhookDbTester.insertGlobalWebhook(); - WebhookDto dto2 = webhookDbTester.insertGlobalWebhook(); + WebhookDto dto2 = webhookDbTester.insertGlobalWebhook().setSecret(null); // insert a project-specific webhook, that should not be returned when listing global webhooks webhookDbTester.insertWebhook(componentDbTester.insertPrivateProject().getProjectDto()); @@ -177,10 +178,26 @@ public class ListActionIT { .executeProtobuf(ListResponse.class); assertThat(response.getWebhooksList()) - .extracting(Webhooks.ListResponseElement::getName, Webhooks.ListResponseElement::getUrl) + .extracting(ListResponseElement::getName, ListResponseElement::getUrl) .containsExactlyInAnyOrder(tuple(dto1.getName(), dto1.getUrl()), tuple(dto2.getName(), dto2.getUrl())); + } + + @Test + public void list_webhooks_with_secret() { + WebhookDto withSecret = webhookDbTester.insertGlobalWebhook(); + WebhookDto withoutSecret = newGlobalWebhook().setSecret(null); + webhookDbTester.insert(withoutSecret, null, null); + + userSession.logIn().addPermission(GlobalPermission.ADMINISTER); + ListResponse response = wsActionTester.newRequest() + .executeProtobuf(ListResponse.class); + + assertThat(response.getWebhooksList()) + .extracting(ListResponseElement::getName, ListResponseElement::getUrl, ListResponseElement::getHasSecret) + .containsExactlyInAnyOrder(tuple(withSecret.getName(), withSecret.getUrl(), true), + tuple(withoutSecret.getName(), withoutSecret.getUrl(), false)); } @Test @@ -196,7 +213,7 @@ public class ListActionIT { .executeProtobuf(ListResponse.class); assertThat(response.getWebhooksList()) - .extracting(Webhooks.ListResponseElement::getName, Webhooks.ListResponseElement::getUrl) + .extracting(ListResponseElement::getName, ListResponseElement::getUrl) .contains(tuple(dto1.getName(), dto1.getUrl()), tuple(dto2.getName(), dto2.getUrl())); @@ -212,7 +229,7 @@ public class ListActionIT { .executeProtobuf(ListResponse.class); assertThat(response.getWebhooksList()) - .extracting(Webhooks.ListResponseElement::getName, Webhooks.ListResponseElement::getUrl) + .extracting(ListResponseElement::getName, ListResponseElement::getUrl) .contains(tuple(dto1.getName(), dto1.getUrl()), tuple(dto2.getName(), dto2.getUrl())); diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/webhook/ws/UpdateActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/webhook/ws/UpdateActionIT.java index 1a68e97da60..ef84925ecda 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/webhook/ws/UpdateActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/webhook/ws/UpdateActionIT.java @@ -104,7 +104,29 @@ public class UpdateActionIT { assertThat(reloaded.get().getName()).isEqualTo(NAME_WEBHOOK_EXAMPLE_001); assertThat(reloaded.get().getUrl()).isEqualTo(URL_WEBHOOK_EXAMPLE_001); assertThat(reloaded.get().getProjectUuid()).isEqualTo(dto.getProjectUuid()); - assertThat(reloaded.get().getSecret()).isNull(); + assertThat(reloaded.get().getSecret()).isEqualTo(dto.getSecret()); + } + + @Test + public void update_with_empty_secrets_removes_the_secret() { + ProjectDto project = componentDbTester.insertPrivateProject().getProjectDto(); + WebhookDto dto = webhookDbTester.insertWebhook(project); + userSession.logIn().addProjectPermission(UserRole.ADMIN, project); + + TestResponse response = wsActionTester.newRequest() + .setParam("webhook", dto.getUuid()) + .setParam("name", NAME_WEBHOOK_EXAMPLE_001) + .setParam("url", URL_WEBHOOK_EXAMPLE_001) + .setParam("secret", "") + .execute(); + + assertThat(response.getStatus()).isEqualTo(HTTP_NO_CONTENT); + Optional reloaded = webhookDbTester.selectWebhook(dto.getUuid()); + assertThat(reloaded).isPresent(); + assertThat(reloaded.get().getName()).isEqualTo(NAME_WEBHOOK_EXAMPLE_001); + assertThat(reloaded.get().getUrl()).isEqualTo(URL_WEBHOOK_EXAMPLE_001); + assertThat(reloaded.get().getProjectUuid()).isEqualTo(dto.getProjectUuid()); + assertThat(reloaded.get().getSecret()).isEqualTo(null); } @Test diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/CreateAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/CreateAction.java index bb956d29f9b..7c951bc1a31 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/CreateAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/CreateAction.java @@ -156,10 +156,8 @@ public class CreateAction implements WebhooksWsAction { webhookBuilder .setKey(dto.getUuid()) .setName(dto.getName()) - .setUrl(dto.getUrl()); - if (dto.getSecret() != null) { - webhookBuilder.setSecret(dto.getSecret()); - } + .setUrl(dto.getUrl()) + .setHasSecret(dto.getSecret() != null); writeProtobuf(newBuilder().setWebhook(webhookBuilder).build(), request, response); } @@ -174,8 +172,8 @@ public class CreateAction implements WebhooksWsAction { } private void checkNumberOfGlobalWebhooks(DbSession dbSession) { - int globalWehbooksCount = dbClient.webhookDao().selectGlobalWebhooks(dbSession).size(); - if (globalWehbooksCount >= MAX_NUMBER_OF_WEBHOOKS) { + int globalWebhooksCount = dbClient.webhookDao().selectGlobalWebhooks(dbSession).size(); + if (globalWebhooksCount >= MAX_NUMBER_OF_WEBHOOKS) { throw new IllegalArgumentException("Maximum number of global webhooks reached"); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/ListAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/ListAction.java index 0f669a6fbb5..16004122c77 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/ListAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/ListAction.java @@ -74,6 +74,7 @@ public class ListAction implements WebhooksWsAction { .setExampleValue(KEY_PROJECT_EXAMPLE_001); action.setChangelog(new Change("7.8", "Field 'secret' added to response")); + action.setChangelog(new Change("10.1", "Field 'secret' replaced by flag 'hasSecret' in response")); } @Override @@ -113,10 +114,8 @@ public class ListAction implements WebhooksWsAction { responseElementBuilder .setKey(webhook.getUuid()) .setName(webhook.getName()) - .setUrl(obfuscateCredentials(webhook.getUrl())); - if (webhook.getSecret() != null) { - responseElementBuilder.setSecret(webhook.getSecret()); - } + .setUrl(obfuscateCredentials(webhook.getUrl())) + .setHasSecret(webhook.getSecret() != null); addLastDelivery(responseElementBuilder, webhook, lastDeliveries); }); writeProtobuf(responseBuilder.build(), request, response); diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/UpdateAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/UpdateAction.java index bf2f8e1020b..63097e2347c 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/UpdateAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/UpdateAction.java @@ -31,6 +31,7 @@ import org.sonar.db.webhook.WebhookDto; import org.sonar.server.component.ComponentFinder; import org.sonar.server.user.UserSession; +import static org.apache.commons.lang.StringUtils.isBlank; import static org.sonar.server.exceptions.NotFoundException.checkFoundWithOptional; import static org.sonar.server.webhook.ws.WebhooksWsParameters.KEY_PARAM; import static org.sonar.server.webhook.ws.WebhooksWsParameters.KEY_PARAM_MAXIMUM_LENGTH; @@ -89,9 +90,9 @@ public class UpdateAction implements WebhooksWsAction { action.createParam(SECRET_PARAM) .setRequired(false) - .setMinimumLength(1) .setMaximumLength(SECRET_PARAM_MAXIMUM_LENGTH) - .setDescription("If provided, secret will be used as the key to generate the HMAC hex (lowercase) digest value in the 'X-Sonar-Webhook-HMAC-SHA256' header") + .setDescription("If provided, secret will be used as the key to generate the HMAC hex (lowercase) digest value in the 'X-Sonar-Webhook-HMAC-SHA256' header. " + + "If blank, any secret previously configured will be removed. If not set, the secret will remain unchanged.") .setExampleValue("your_secret") .setSince("7.8"); } @@ -100,7 +101,7 @@ public class UpdateAction implements WebhooksWsAction { public void handle(Request request, Response response) throws Exception { userSession.checkLoggedIn(); - String webhookKey = request.param(KEY_PARAM); + String webhookKey = request.mandatoryParam(KEY_PARAM); String name = request.mandatoryParam(NAME_PARAM); String url = request.mandatoryParam(URL_PARAM); String secret = request.param(SECRET_PARAM); @@ -132,9 +133,30 @@ public class UpdateAction implements WebhooksWsAction { @Nullable String projectKey, @Nullable String projectName) { dto .setName(name) - .setUrl(url) - .setSecret(secret); + .setUrl(url); + setSecret(dto, secret); dbClient.webhookDao().update(dbSession, dto, projectKey, projectName); } + /** + *

Sets the secret of the webhook. The secret is set according to the following rules: + *

    + *
  • If the secret is null, it will remain unchanged.
    • + *
  • If the secret is blank (""), it will be removed.
    • + *
  • If the secret is not null or blank, it will be set to the provided value.
    • + *
+ *

+ * @param dto The webhook to update. It holds the old secret value. + * @param secret The new secret value. It can be null or blank. + */ + private static void setSecret(WebhookDto dto, @Nullable String secret) { + if (secret != null) { + if (isBlank(secret)) { + dto.setSecret(null); + } else { + dto.setSecret(secret); + } + } + } + } diff --git a/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/webhook/ws/example-webhook-create.json b/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/webhook/ws/example-webhook-create.json index 29b4ee4762e..9aa62a4cf75 100644 --- a/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/webhook/ws/example-webhook-create.json +++ b/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/webhook/ws/example-webhook-create.json @@ -3,6 +3,6 @@ "key": "uuid", "name": "My webhook", "url": "https://www.my-webhook-listener.com/sonar", - "secret": "your_secret" + "hasSecret": true } } diff --git a/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/webhook/ws/example-webhooks-list.json b/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/webhook/ws/example-webhooks-list.json index 008bfd0f624..6118def59c2 100644 --- a/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/webhook/ws/example-webhooks-list.json +++ b/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/webhook/ws/example-webhooks-list.json @@ -3,13 +3,14 @@ { "key": "UUID-1", "name": "my first webhook", - "url": "http://www.my-webhook-listener.com/sonarqube" + "url": "http://www.my-webhook-listener.com/sonarqube", + "hasSecret": "false" }, { "key": "UUID-2", "name": "my 2nd webhook", "url": "https://www.my-other-webhook-listener.com/fancy-listner", - "secret": "your_secret" + "hasSecret": "true" } ] } diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties index e65f69220c8..5aacd5992a6 100644 --- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties +++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties @@ -4757,9 +4757,12 @@ webhooks.name.required=Name is required. webhooks.no_result=No webhook defined. webhooks.update=Update Webhook webhooks.secret=Secret -webhooks.secret_header=Secret? +webhooks.secret_header=Has secret? webhooks.secret.bad_format=Secret must have a maximum length of 200 characters -webhooks.secret.description=If provided, secret will be used as the key to generate the HMAC hex (lowercase) digest value in the 'X-Sonar-Webhook-HMAC-SHA256' header +webhooks.secret.description=If provided, secret will be used as the key to generate the HMAC hex (lowercase) digest value in the 'X-Sonar-Webhook-HMAC-SHA256' header. +webhooks.secret.description.update=If blank, any secret previously configured will be removed. If not set, the secret will remain unchanged. +webhooks.secret.field_mask.description=Hidden for security reasons. +webhooks.secret.field_mask.link=Click here to update the secret webhooks.url=URL webhooks.url.bad_format=Bad format of URL. webhooks.url.bad_protocol=URL must start with "http://" or "https://". diff --git a/sonar-ws/src/main/protobuf/ws-webhooks.proto b/sonar-ws/src/main/protobuf/ws-webhooks.proto index b67f893791f..256ef05d2a7 100644 --- a/sonar-ws/src/main/protobuf/ws-webhooks.proto +++ b/sonar-ws/src/main/protobuf/ws-webhooks.proto @@ -39,7 +39,9 @@ message ListResponseElement { optional string name = 2; optional string url = 3; optional LatestDelivery latestDelivery = 4; - optional string secret = 5; + // deprecated + // optional string secret = 5; + optional bool hasSecret = 6; } // GET api/webhooks/list @@ -55,7 +57,9 @@ message CreateWsResponse { optional string key = 1; optional string name = 2; optional string url = 3; - optional string secret = 4; + // deprecated + // optional string secret = 4; + required bool hasSecret = 5; } } -- 2.39.5