SONAR-18835 Make webhook secret private (#8479)

Co-authored-by: Ambroise C <ambroise.christea@sonarsource.com>

(cherry picked from commit 441a87e76b)

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<void | Respons
   return post('/api/webhooks/delete', data).catch(throwGlobalError);
 }
 
-export function searchWebhooks(data: { project?: string }): Promise<{ webhooks: Webhook[] }> {
+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<void | Response> {
+export function updateWebhook(data: WebhookUpdatePayload): Promise<void | Response> {
   return post('/api/webhooks/update', data).catch(throwGlobalError);
 }
 

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<Props, State> {
@@ -99,19 +99,24 @@ export class App extends React.PureComponent<Props, State> {
   };
 
   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
           ),
         }));

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<void>;
-  webhook?: Webhook;
-}
-
-interface Values {
-  name: string;
-  secret: string;
-  url: string;
+  onDone: (data: WebhookBasePayload) => Promise<void>;
+  webhook?: WebhookResponse;
 }
 
 export default class CreateWebhookForm extends React.PureComponent<Props> {
@@ -45,7 +40,7 @@ export default class CreateWebhookForm extends React.PureComponent<Props> {
     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<Props> {
         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<Props> {
               type="text"
               value={values.url}
             />
-            <InputValidationField
-              description={translate('webhooks.secret.description')}
+            <UpdateWebhookSecretField
+              description={`${translate('webhooks.secret.description')}${
+                isUpdate ? ` ${translate('webhooks.secret.description.update')}` : ''
+              }`}
               dirty={dirty}
               disabled={isSubmitting}
               error={errors.secret}
               id="webhook-secret"
+              isUpdateForm={isUpdate}
               label={<label htmlFor="webhook-secret">{translate('webhooks.secret')}</label>}
               name="secret"
               onBlur={handleBlur}

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<void>;
-  webhook: Webhook;
+  webhook: WebhookResponse;
 }
 
 export default function DeleteWebhookForm({ onClose, onSubmit, webhook }: Props) {

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 {

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 {

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<HTMLInputElement>) => void;
+  onChange: (event: React.ChangeEvent<HTMLInputElement>) => 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 ? (
+    <InputValidationField
+      description={description}
+      dirty={dirty}
+      disabled={disabled}
+      error={error}
+      id={id}
+      label={label}
+      name={name}
+      onBlur={onBlur}
+      onChange={onChange}
+      touched={touched}
+      type={type}
+      value={value as string}
+    />
+  ) : (
+    <ModalValidationField
+      description={description}
+      dirty={false}
+      error={undefined}
+      label={label}
+      touched={true}
+    >
+      {() => (
+        <div className="sw-mb-5 sw-leading-6 sw-flex sw-items-center">
+          <span className="sw-mr-1/2">{translate('webhooks.secret.field_mask.description')}</span>
+          <ButtonLink onClick={showSecretInput}>
+            {translate('webhooks.secret.field_mask.link')}
+          </ButtonLink>
+        </div>
+      )}
+    </ModalValidationField>
+  );
+}

server/sonar-web/src/main/js/apps/webhooks/components/WebhookActions.tsx

@@ -23,15 +23,15 @@ import ActionsDropdown, {
   ActionsDropdownItem,
 } from '../../../components/controls/ActionsDropdown';
 import { translate } 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<void>;
-  onUpdate: (data: { webhook: string; name: string; url: string }) => Promise<void>;
-  webhook: Webhook;
+  onUpdate: (data: WebhookUpdatePayload) => Promise<void>;
+  webhook: WebhookResponse;
 }
 
 interface State {
@@ -74,7 +74,7 @@ export default class WebhookActions extends React.PureComponent<Props, State> {
     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 });
   };
 

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<void>;
-  onUpdate: (data: { webhook: string; name: string; url: string }) => Promise<void>;
-  webhook: Webhook;
+  onUpdate: (data: WebhookUpdatePayload) => Promise<void>;
+  webhook: WebhookResponse;
 }
 
 export default function WebhookItem({ onDelete, onUpdate, webhook }: Props) {
@@ -34,11 +34,11 @@ export default function WebhookItem({ onDelete, onUpdate, webhook }: Props) {
     <tr>
       <td>{webhook.name}</td>
       <td>{webhook.url}</td>
-      <td>{webhook.secret ? translate('yes') : translate('no')}</td>
+      <td>{webhook.hasSecret ? translate('yes') : translate('no')}</td>
       <td>
         <WebhookItemLatestDelivery webhook={webhook} />
       </td>
-      <td className="thin nowrap text-right">
+      <td className="sw-text-right">
         <WebhookActions onDelete={onDelete} onUpdate={onUpdate} webhook={webhook} />
       </td>
     </tr>

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 {

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<void>;
-  onUpdate: (data: { webhook: string; name: string; url: string }) => Promise<void>;
-  webhooks: Webhook[];
+  onUpdate: (data: WebhookUpdatePayload) => Promise<void>;
+  webhooks: WebhookResponse[];
 }
 
 export default class WebhooksList extends React.PureComponent<Props> {
@@ -37,7 +37,7 @@ export default class WebhooksList extends React.PureComponent<Props> {
         <th>{translate('webhooks.url')}</th>
         <th>{translate('webhooks.secret_header')}</th>
         <th>{translate('webhooks.last_execution')}</th>
-        <th />
+        <th className="sw-text-right">{translate('actions')}</th>
       </tr>
     </thead>
   );

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(<App />);
   (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(<App />);
+
+  // 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 },
   ]);
 });

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(
+    <CreateWebhookForm onClose={jest.fn()} onDone={jest.fn(() => Promise.resolve())} {...props} />
+  );
+}

server/sonar-web/src/main/js/apps/webhooks/components/__tests__/CreateWebhookForm-test.tsx

@@ -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(
-    <CreateWebhookForm onClose={jest.fn()} onDone={jest.fn(() => Promise.resolve())} {...props} />
-  );
-}

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<any>).mockClear();

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<any>).mockClear();

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 = {

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', () => {
       <WebhookItem
         onDelete={jest.fn(() => Promise.resolve())}
         onUpdate={jest.fn(() => Promise.resolve())}
-        webhook={webhook}
+        webhook={webhookWithoutSecret}
+      />
+    )
+  ).toMatchSnapshot();
+  expect(
+    shallow(
+      <WebhookItem
+        onDelete={jest.fn(() => Promise.resolve())}
+        onUpdate={jest.fn(() => Promise.resolve())}
+        webhook={webhookWithSecret}
       />
     )
   ).toMatchSnapshot();

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,
 };
 

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', () => {

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",

server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/CreateWebhookForm-test.tsx.snap

@@ -1,61 +0,0 @@
-// Jest Snapshot v1, https://goo.gl/fbAQLP
-
-exports[`should render correctly when creating a new webhook 1`] = `
-<ValidationModal
-  confirmButtonText="create"
-  header="webhooks.create"
-  initialValues={
-    {
-      "name": "",
-      "secret": "",
-      "url": "",
-    }
-  }
-  onClose={[MockFunction]}
-  onSubmit={[MockFunction]}
-  size="small"
-  validate={[Function]}
->
-  <Component />
-</ValidationModal>
-`;
-
-exports[`should render correctly when updating a webhook with a secret 1`] = `
-<ValidationModal
-  confirmButtonText="update_verb"
-  header="webhooks.update"
-  initialValues={
-    {
-      "name": "bar",
-      "secret": "sonar",
-      "url": "http://foo.bar",
-    }
-  }
-  onClose={[MockFunction]}
-  onSubmit={[MockFunction]}
-  size="small"
-  validate={[Function]}
->
-  <Component />
-</ValidationModal>
-`;
-
-exports[`should render correctly when updating a webhook without secret 1`] = `
-<ValidationModal
-  confirmButtonText="update_verb"
-  header="webhooks.update"
-  initialValues={
-    {
-      "name": "foo",
-      "secret": "",
-      "url": "http://foo.bar",
-    }
-  }
-  onClose={[MockFunction]}
-  onSubmit={[MockFunction]}
-  size="small"
-  validate={[Function]}
->
-  <Component />
-</ValidationModal>
-`;

server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/WebhookItem-test.tsx.snap

@@ -15,6 +15,7 @@ exports[`should render correctly 1`] = `
     <WebhookItemLatestDelivery
       webhook={
         {
+          "hasSecret": false,
           "key": "1",
           "name": "my webhook",
           "url": "http://webhook.target",
@@ -23,13 +24,56 @@ exports[`should render correctly 1`] = `
     />
   </td>
   <td
-    className="thin nowrap text-right"
+    className="sw-text-right"
   >
     <WebhookActions
       onDelete={[MockFunction]}
       onUpdate={[MockFunction]}
       webhook={
         {
+          "hasSecret": false,
+          "key": "1",
+          "name": "my webhook",
+          "url": "http://webhook.target",
+        }
+      }
+    />
+  </td>
+</tr>
+`;
+
+exports[`should render correctly 2`] = `
+<tr>
+  <td>
+    my webhook
+  </td>
+  <td>
+    http://webhook.target
+  </td>
+  <td>
+    yes
+  </td>
+  <td>
+    <WebhookItemLatestDelivery
+      webhook={
+        {
+          "hasSecret": true,
+          "key": "1",
+          "name": "my webhook",
+          "url": "http://webhook.target",
+        }
+      }
+    />
+  </td>
+  <td
+    className="sw-text-right"
+  >
+    <WebhookActions
+      onDelete={[MockFunction]}
+      onUpdate={[MockFunction]}
+      webhook={
+        {
+          "hasSecret": true,
           "key": "1",
           "name": "my webhook",
           "url": "http://webhook.target",

server/sonar-web/src/main/js/apps/webhooks/components/__tests__/__snapshots__/WebhooksList-test.tsx.snap

@@ -24,7 +24,11 @@ exports[`should correctly render the webhooks 1`] = `
       <th>
         webhooks.last_execution
       </th>
-      <th />
+      <th
+        className="sw-text-right"
+      >
+        actions
+      </th>
     </tr>
   </thead>
   <tbody>
@@ -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",

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;

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");
     }
   }

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);

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);
   }
 
+  /**
+   * <p>Sets the secret of the webhook. The secret is set according to the following rules:
+   * <ul>
+   *   <li>If the secret is null, it will remain unchanged.</li>
+   *   <li>If the secret is blank (""), it will be removed.</li>
+   *   <li>If the secret is not null or blank, it will be set to the provided value.</li>
+   * </ul>
+   * </p>
+   * @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);
+      }
+    }
+  }
+
 }

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
   }
 }

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"
     }
   ]
 }

server/sonar-webserver-webapi/src/test/java/org/sonar/server/webhook/ws/CreateActionTest.java

@@ -111,7 +111,7 @@ public class CreateActionTest {
     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 CreateActionTest {
     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 CreateActionTest {
     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 CreateActionTest {
     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 CreateActionTest {
   }
 
   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));
   }
 
 }

server/sonar-webserver-webapi/src/test/java/org/sonar/server/webhook/ws/ListActionTest.java

@@ -30,6 +30,7 @@ import org.sonar.db.DbClient;
 import org.sonar.db.DbTester;
 import org.sonar.db.component.ComponentDbTester;
 import org.sonar.db.component.ComponentDto;
+import org.sonar.db.permission.GlobalPermission;
 import org.sonar.db.project.ProjectDto;
 import org.sonar.db.webhook.WebhookDbTester;
 import org.sonar.db.webhook.WebhookDeliveryDbTester;
@@ -41,7 +42,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 +55,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 ListActionTest {
 
@@ -92,7 +94,7 @@ public class ListActionTest {
       .extracting(Param::key, Param::isRequired)
       .containsExactlyInAnyOrder(
         tuple("project", false));
-    assertThat(action.changelog()).hasSize(1);
+    assertThat(action.changelog()).hasSize(2);
   }
 
   @Test
@@ -109,18 +111,18 @@ public class ListActionTest {
 
     ListResponse response = wsActionTester.newRequest().executeProtobuf(ListResponse.class);
 
-    List<Webhooks.ListResponseElement> elements = response.getWebhooksList();
+    List<ListResponseElement> 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 +134,15 @@ public class ListActionTest {
 
     ListResponse response = wsActionTester.newRequest().executeProtobuf(ListResponse.class);
 
-    List<Webhooks.ListResponseElement> elements = response.getWebhooksList();
+    List<ListResponseElement> 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 +159,17 @@ public class ListActionTest {
 
     ListResponse response = wsActionTester.newRequest().executeProtobuf(ListResponse.class);
 
-    List<Webhooks.ListResponseElement> elements = response.getWebhooksList();
+    List<ListResponseElement> 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.insertPrivateProjectDto());
 
@@ -177,10 +179,26 @@ public class ListActionTest {
       .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 +214,7 @@ public class ListActionTest {
       .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 +230,7 @@ public class ListActionTest {
       .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()));
 

server/sonar-webserver-webapi/src/test/java/org/sonar/server/webhook/ws/UpdateActionTest.java

@@ -25,6 +25,7 @@ import org.junit.Test;
 import org.sonar.api.config.Configuration;
 import org.sonar.api.resources.ResourceTypes;
 import org.sonar.api.server.ws.WebService;
+import org.sonar.api.web.UserRole;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbTester;
 import org.sonar.db.component.ComponentDbTester;
@@ -104,7 +105,29 @@ public class UpdateActionTest {
     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.insertPrivateProjectDto();
+    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<WebhookDto> 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

sonar-core/src/main/resources/org/sonar/l10n/core.properties

@@ -4473,9 +4473,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://".

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;
   }
 }