]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-17266 Add support of webhook secret for api/alm_settings/(create_github|update_...
authorAntoine Vinot <antoine.vinot@sonarsource.com>
Mon, 5 Sep 2022 14:50:31 +0000 (16:50 +0200)
committersonartech <sonartech@sonarsource.com>
Tue, 13 Sep 2022 20:03:07 +0000 (20:03 +0000)
server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDto.java
server/sonar-db-dao/src/main/resources/org/sonar/db/alm/setting/AlmSettingMapper.xml
server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoTest.java
server/sonar-db-dao/src/testFixtures/java/org/sonar/db/almsettings/AlmSettingsTesting.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/CreateGithubAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateGithubAction.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/almsettings/ws/CreateGithubActionTest.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/almsettings/ws/UpdateGithubActionTest.java

index c297b1c5e31f98cb26e14263f08342172c8baf29..d2c8dd391644a3df6a7edb923e6616da4a310c54 100644 (file)
@@ -78,6 +78,7 @@ public class AlmSettingDto {
    * It will be null when the ALM is Azure DevOps or Bitbucket.
    */
   private String clientId;
+
   /**
    * Application client secret of the GitHub instance. Max size is 80.
    * This column will only be fed when alm is GitHub.
@@ -85,6 +86,13 @@ public class AlmSettingDto {
    */
   private String clientSecret;
 
+  /**
+   * Webhook secret of the GitHub instance. Max size is 160.
+   * This column will only be fed when alm is GitHub.
+   * It will be null when the ALM is Azure DevOps or Bitbucket.
+   */
+  private String webhookSecret;
+
   private long updatedAt;
   private long createdAt;
 
@@ -193,6 +201,19 @@ public class AlmSettingDto {
     return this;
   }
 
+  @CheckForNull
+  public String getDecryptedWebhookSecret(Encryption encryption) {
+    if (!isNullOrEmpty(webhookSecret) && encryption.isEncrypted(webhookSecret)) {
+      return encryption.decrypt(webhookSecret);
+    }
+    return webhookSecret;
+  }
+
+  public AlmSettingDto setWebhookSecret(@Nullable String webhookSecret) {
+    this.webhookSecret = webhookSecret;
+    return this;
+  }
+
   public long getUpdatedAt() {
     return updatedAt;
   }
index ee93b8c97b25715a977edf1f2f2cf9a497f78936..2e45e8bc34cc4188a87b72fbd92edfcc3cf03507 100644 (file)
@@ -14,7 +14,8 @@
     a.client_id as "clientId",
     a.client_secret as "clientSecret",
     a.created_at as "createdAt",
-    a.updated_at as "updatedAt"
+    a.updated_at as "updatedAt",
+    a.webhook_secret as "webhookSecret"
   </sql>
 
   <select id="selectByUuid" parameterType="string" resultType="org.sonar.db.alm.setting.AlmSettingDto">
@@ -60,7 +61,8 @@
       client_id,
       client_secret,
       created_at,
-      updated_at
+      updated_at,
+      webhook_secret
     )
     VALUES (
       #{dto.uuid, jdbcType=VARCHAR},
@@ -73,7 +75,8 @@
       #{dto.clientId, jdbcType=VARCHAR},
       #{dto.clientSecret, jdbcType=VARCHAR},
       #{dto.createdAt, jdbcType=BIGINT},
-      #{dto.updatedAt, jdbcType=BIGINT}
+      #{dto.updatedAt, jdbcType=BIGINT},
+      #{dto.webhookSecret, jdbcType=VARCHAR}
     )
   </insert>
 
@@ -87,7 +90,8 @@
       pat = #{dto.personalAccessToken, jdbcType=VARCHAR},
       client_id = #{dto.clientId, jdbcType=VARCHAR},
       client_secret = #{dto.clientSecret, jdbcType=VARCHAR},
-      updated_at = #{dto.updatedAt, jdbcType=BIGINT}
+      updated_at = #{dto.updatedAt, jdbcType=BIGINT},
+      webhook_secret = #{dto.webhookSecret, jdbcType=VARCHAR}
     </set>
     <where>
       uuid = #{dto.uuid, jdbcType=VARCHAR}
index f130100f24b6f92088b96b89f7649f8a7323f369..74e3c69a2380b2d899762d3fa11fe6902e32f706 100644 (file)
 package org.sonar.db.alm.setting;
 
 import java.util.List;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
-import org.sonar.api.config.internal.Encryption;
 import org.sonar.api.impl.utils.TestSystem2;
 import org.sonar.core.util.UuidFactory;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
-import org.sonar.db.almsettings.AlmSettingsTesting;
 import org.sonar.db.audit.NoOpAuditPersister;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
+import static org.sonar.db.alm.setting.ALM.GITHUB;
+import static org.sonar.db.almsettings.AlmSettingsTesting.newAlmSettingDtoWithEmptySecrets;
 import static org.sonar.db.almsettings.AlmSettingsTesting.newGithubAlmSettingDto;
+import static org.sonar.db.almsettings.AlmSettingsTesting.newGithubAlmSettingDtoWithNonRequiredField;
 
 public class AlmSettingDaoTest {
 
   private static final long NOW = 1000000L;
   private static final String A_UUID = "SOME_UUID";
-  private TestSystem2 system2 = new TestSystem2().setNow(NOW);
+  private final TestSystem2 system2 = new TestSystem2().setNow(NOW);
   @Rule
   public DbTester db = DbTester.create(system2);
 
-  private final Encryption encryption = mock(Encryption.class);
+  private final DbSession dbSession = db.getSession();
+  private final UuidFactory uuidFactory = mock(UuidFactory.class);
 
-  private DbSession dbSession = db.getSession();
-  private UuidFactory uuidFactory = mock(UuidFactory.class);
+  private final AlmSettingDao underTest = new AlmSettingDao(system2, uuidFactory, new NoOpAuditPersister());
 
-  private AlmSettingDao underTest = new AlmSettingDao(system2, uuidFactory, new NoOpAuditPersister());
+  @Before
+  public void setUp() {
+    when(uuidFactory.create()).thenReturn(A_UUID);
+  }
 
   @Test
   public void selectByUuid() {
-    when(uuidFactory.create()).thenReturn(A_UUID);
-    when(encryption.isEncrypted(any())).thenReturn(false);
+    AlmSettingDto expected = newGithubAlmSettingDtoWithNonRequiredField();
+    underTest.insert(dbSession, expected);
 
-    AlmSettingDto almSettingDto = newGithubAlmSettingDto();
-    underTest.insert(dbSession, almSettingDto);
+    AlmSettingDto result = underTest.selectByUuid(dbSession, A_UUID).orElse(null);
+
+    assertThat(result).usingRecursiveComparison().isEqualTo(expected);
+  }
 
-    assertThat(underTest.selectByUuid(dbSession, A_UUID).get())
-      .extracting(AlmSettingDto::getUuid, AlmSettingDto::getKey, AlmSettingDto::getRawAlm, AlmSettingDto::getUrl,
-        AlmSettingDto::getAppId, AlmSettingDto::getCreatedAt, AlmSettingDto::getUpdatedAt,
-        s -> almSettingDto.getDecryptedPrivateKey(encryption),
-        s -> almSettingDto.getDecryptedPersonalAccessToken(encryption),
-        s -> almSettingDto.getDecryptedClientSecret(encryption))
-      .containsExactly(A_UUID, almSettingDto.getKey(), ALM.GITHUB.getId(), almSettingDto.getUrl(),
-        almSettingDto.getAppId(), NOW, NOW,
-        almSettingDto.getDecryptedPrivateKey(encryption),
-        almSettingDto.getDecryptedPersonalAccessToken(encryption),
-        almSettingDto.getDecryptedClientSecret(encryption));
+  @Test
+  public void selectByUuid_shouldNotFindResult_whenUuidIsNotPresent() {
+    AlmSettingDto expected = newGithubAlmSettingDtoWithNonRequiredField();
+    underTest.insert(dbSession, expected);
 
     assertThat(underTest.selectByUuid(dbSession, "foo")).isNotPresent();
   }
 
   @Test
   public void selectByKey() {
-    when(uuidFactory.create()).thenReturn(A_UUID);
-    String decrypted = "decrypted";
-    when(encryption.isEncrypted(any())).thenReturn(true);
-    when(encryption.decrypt(any())).thenReturn(decrypted);
+    AlmSettingDto expected = newGithubAlmSettingDtoWithNonRequiredField();
+    underTest.insert(dbSession, expected);
 
-    AlmSettingDto almSettingDto = AlmSettingsTesting.newGithubAlmSettingDto();
-    underTest.insert(dbSession, almSettingDto);
+    AlmSettingDto result = underTest.selectByKey(dbSession, expected.getKey()).orElse(null);
 
-    assertThat(underTest.selectByKey(dbSession, almSettingDto.getKey()).get())
-      .extracting(AlmSettingDto::getUuid, AlmSettingDto::getKey, AlmSettingDto::getRawAlm, AlmSettingDto::getUrl,
-        AlmSettingDto::getAppId, AlmSettingDto::getCreatedAt, AlmSettingDto::getUpdatedAt,
-        s -> almSettingDto.getDecryptedPrivateKey(encryption),
-        s -> almSettingDto.getDecryptedPersonalAccessToken(encryption),
-        s -> almSettingDto.getDecryptedClientSecret(encryption))
-      .containsExactly(A_UUID, almSettingDto.getKey(), ALM.GITHUB.getId(), almSettingDto.getUrl(),
-        almSettingDto.getAppId(), NOW, NOW,
-        almSettingDto.getDecryptedPrivateKey(encryption),
-        null,
-        almSettingDto.getDecryptedClientSecret(encryption));
+    assertThat(result).usingRecursiveComparison().isEqualTo(expected);
+  }
+
+  @Test
+  public void selectByKey_shouldNotFindResult_whenKeyIsNotPresent() {
+    AlmSettingDto expected = newGithubAlmSettingDtoWithNonRequiredField();
+    underTest.insert(dbSession, expected);
 
     assertThat(underTest.selectByKey(dbSession, "foo")).isNotPresent();
   }
 
   @Test
   public void selectByKey_withEmptySecrets() {
-    when(uuidFactory.create()).thenReturn(A_UUID);
-    String decrypted = "decrypted";
-    when(encryption.isEncrypted(any())).thenReturn(true);
-    when(encryption.decrypt(any())).thenReturn(decrypted);
-
-    AlmSettingDto almSettingDto = AlmSettingsTesting.newAlmSettingDtoWithEmptySecrets();
-    underTest.insert(dbSession, almSettingDto);
+    AlmSettingDto expected = newAlmSettingDtoWithEmptySecrets();
+    underTest.insert(dbSession, expected);
 
-    assertThat(underTest.selectByKey(dbSession, almSettingDto.getKey()).get())
-      .extracting(AlmSettingDto::getUuid, AlmSettingDto::getKey, AlmSettingDto::getRawAlm, AlmSettingDto::getUrl,
-        AlmSettingDto::getAppId, AlmSettingDto::getCreatedAt, AlmSettingDto::getUpdatedAt,
-        s -> almSettingDto.getDecryptedPrivateKey(encryption),
-        s -> almSettingDto.getDecryptedPersonalAccessToken(encryption),
-        s -> almSettingDto.getDecryptedClientSecret(encryption))
-      .containsExactly(A_UUID, almSettingDto.getKey(), ALM.GITHUB.getId(), almSettingDto.getUrl(),
-        almSettingDto.getAppId(), NOW, NOW, null, null, null);
+    AlmSettingDto result = underTest.selectByKey(dbSession, expected.getKey()).orElse(null);
 
-    assertThat(underTest.selectByKey(dbSession, "foo")).isNotPresent();
+    assertThat(result).usingRecursiveComparison().isEqualTo(expected);
   }
 
   @Test
   public void selectByAlm() {
-    when(uuidFactory.create()).thenReturn(A_UUID);
     AlmSettingDto gitHubAlmSetting1 = db.almSettings().insertGitHubAlmSetting();
     AlmSettingDto gitHubAlmSetting2 = db.almSettings().insertGitHubAlmSetting();
-    AlmSettingDto azureAlmSetting2 = db.almSettings().insertAzureAlmSetting();
+    db.almSettings().insertAzureAlmSetting();
 
-    List<AlmSettingDto> almSettings = underTest.selectByAlm(dbSession, ALM.GITHUB);
+    List<AlmSettingDto> almSettings = underTest.selectByAlm(dbSession, GITHUB);
 
     assertThat(almSettings)
       .extracting(AlmSettingDto::getUuid)
@@ -137,7 +116,6 @@ public class AlmSettingDaoTest {
 
   @Test
   public void selectAll() {
-    when(uuidFactory.create()).thenReturn(A_UUID);
     underTest.insert(dbSession, newGithubAlmSettingDto());
     when(uuidFactory.create()).thenReturn(A_UUID + "bis");
     underTest.insert(dbSession, newGithubAlmSettingDto());
@@ -149,34 +127,27 @@ public class AlmSettingDaoTest {
 
   @Test
   public void update() {
-    when(uuidFactory.create()).thenReturn(A_UUID);
-    AlmSettingDto almSettingDto = newGithubAlmSettingDto();
-    underTest.insert(dbSession, almSettingDto);
+    //GIVEN
+    AlmSettingDto expected = newGithubAlmSettingDto();
+    underTest.insert(dbSession, expected);
 
-    almSettingDto.setPrivateKey("updated private key");
-    almSettingDto.setAppId("updated app id");
-    almSettingDto.setUrl("updated url");
-    almSettingDto.setPersonalAccessToken("updated pat");
-    almSettingDto.setKey("updated key");
+    expected.setPrivateKey("updated private key");
+    expected.setAppId("updated app id");
+    expected.setUrl("updated url");
+    expected.setPersonalAccessToken("updated pat");
+    expected.setKey("updated key");
+    expected.setWebhookSecret("updated webhook secret");
 
     system2.setNow(NOW + 1);
-    underTest.update(dbSession, almSettingDto, false);
-
-    AlmSettingDto result = underTest.selectByUuid(dbSession, A_UUID).get();
-    assertThat(result)
-      .extracting(AlmSettingDto::getUuid, AlmSettingDto::getKey, AlmSettingDto::getRawAlm, AlmSettingDto::getUrl,
-        AlmSettingDto::getAppId,
-        s -> almSettingDto.getDecryptedPrivateKey(encryption),
-        s -> almSettingDto.getDecryptedPersonalAccessToken(encryption),
-        AlmSettingDto::getCreatedAt, AlmSettingDto::getUpdatedAt)
-      .containsExactly(A_UUID, almSettingDto.getKey(), ALM.GITHUB.getId(), almSettingDto.getUrl(),
-        almSettingDto.getAppId(), almSettingDto.getDecryptedPrivateKey(encryption),
-        almSettingDto.getDecryptedPersonalAccessToken(encryption), NOW, NOW + 1);
+    //WHEN
+    underTest.update(dbSession, expected, false);
+    //THEN
+    AlmSettingDto result = underTest.selectByUuid(dbSession, A_UUID).orElse(null);
+    assertThat(result).usingRecursiveComparison().isEqualTo(expected);
   }
 
   @Test
   public void delete() {
-    when(uuidFactory.create()).thenReturn(A_UUID);
     AlmSettingDto almSettingDto = newGithubAlmSettingDto();
     underTest.insert(dbSession, almSettingDto);
 
index 970df468b00d7183124d32991d9411ba829f4f46..8745acb1631ae1b5076a41babc65c7427ffbdd73 100644 (file)
@@ -29,6 +29,14 @@ import static org.apache.commons.lang.RandomStringUtils.randomNumeric;
 
 public class AlmSettingsTesting {
 
+  private AlmSettingsTesting() {
+
+  }
+
+  public static AlmSettingDto newGithubAlmSettingDtoWithNonRequiredField() {
+    return newGithubAlmSettingDto().setWebhookSecret(randomAlphanumeric(160));
+  }
+
   public static AlmSettingDto newGithubAlmSettingDto() {
     return new AlmSettingDto()
       .setKey(randomAlphanumeric(200))
index 6898a8a8ae843a62f610a8ebeeb6f32e00de79ce..cf7b7cd11ec56b6b2e9a49a43821d46c86da59b1 100644 (file)
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.almsettings.ws;
 
+import org.sonar.api.server.ws.Change;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
@@ -27,6 +28,7 @@ import org.sonar.db.DbSession;
 import org.sonar.db.alm.setting.AlmSettingDto;
 import org.sonar.server.user.UserSession;
 
+import static org.apache.commons.lang.StringUtils.removeEnd;
 import static org.sonar.db.alm.setting.ALM.GITHUB;
 
 public class CreateGithubAction implements AlmSettingsWsAction {
@@ -37,6 +39,7 @@ public class CreateGithubAction implements AlmSettingsWsAction {
   private static final String PARAM_CLIENT_ID = "clientId";
   private static final String PARAM_CLIENT_SECRET = "clientSecret";
   private static final String PARAM_PRIVATE_KEY = "privateKey";
+  private static final String PARAM_WEBHOOK_SECRET = "webhookSecret";
 
   private final DbClient dbClient;
   private final UserSession userSession;
@@ -54,6 +57,7 @@ public class CreateGithubAction implements AlmSettingsWsAction {
     WebService.NewAction action = context.createAction("create_github")
       .setDescription("Create GitHub ALM instance Setting. <br/>" +
         "Requires the 'Administer System' permission")
+      .setChangelog(new Change("9.7", String.format("Optional parameter '%s' was added", PARAM_WEBHOOK_SECRET)))
       .setPost(true)
       .setSince("8.1")
       .setHandler(this);
@@ -82,40 +86,41 @@ public class CreateGithubAction implements AlmSettingsWsAction {
       .setRequired(true)
       .setMaximumLength(160)
       .setDescription("GitHub App Client Secret");
+    action.createParam(PARAM_WEBHOOK_SECRET)
+      .setRequired(false)
+      .setMaximumLength(160)
+      .setDescription("GitHub App Webhook Secret");
   }
 
   @Override
   public void handle(Request request, Response response) {
     userSession.checkIsSystemAdministrator();
-    doHandle(request);
+    tryDoHandle(request);
     response.noContent();
   }
 
-  private void doHandle(Request request) {
+  private void tryDoHandle(Request request) {
+    try (DbSession dbSession = dbClient.openSession(false)) {
+      doHandle(request, dbSession);
+    }
+  }
+
+  private void doHandle(Request request, DbSession dbSession) {
+    almSettingsSupport.checkAlmMultipleFeatureEnabled(GITHUB);
     String key = request.mandatoryParam(PARAM_KEY);
-    String url = request.mandatoryParam(PARAM_URL);
-    String appId = request.mandatoryParam(PARAM_APP_ID);
-    String clientId = request.mandatoryParam(PARAM_CLIENT_ID);
-    String clientSecret = request.mandatoryParam(PARAM_CLIENT_SECRET);
-    String privateKey = request.mandatoryParam(PARAM_PRIVATE_KEY);
+    almSettingsSupport.checkAlmSettingDoesNotAlreadyExist(dbSession, key);
 
-    if (url.endsWith("/")) {
-      url = url.substring(0, url.length() - 1);
-    }
+    dbClient.almSettingDao().insert(dbSession, new AlmSettingDto()
+      .setAlm(GITHUB)
+      .setKey(key)
+      .setUrl(removeEnd(request.mandatoryParam(PARAM_URL), "/"))
+      .setAppId(request.mandatoryParam(PARAM_APP_ID))
+      .setPrivateKey(request.mandatoryParam(PARAM_PRIVATE_KEY))
+      .setClientId(request.mandatoryParam(PARAM_CLIENT_ID))
+      .setClientSecret(request.mandatoryParam(PARAM_CLIENT_SECRET))
+      .setWebhookSecret(request.getParam(PARAM_WEBHOOK_SECRET).emptyAsNull().or(() -> null)));
 
-    try (DbSession dbSession = dbClient.openSession(false)) {
-      almSettingsSupport.checkAlmMultipleFeatureEnabled(GITHUB);
-      almSettingsSupport.checkAlmSettingDoesNotAlreadyExist(dbSession, key);
-      dbClient.almSettingDao().insert(dbSession, new AlmSettingDto()
-        .setAlm(GITHUB)
-        .setKey(key)
-        .setUrl(url)
-        .setAppId(appId)
-        .setPrivateKey(privateKey)
-        .setClientId(clientId)
-        .setClientSecret(clientSecret));
-      dbSession.commit();
-    }
+    dbSession.commit();
   }
 
 }
index 70f83b3b23ea081e0127466358f21e8524868c33..b6d1ecb4e08c9be47f730d07b278cb333f067e4f 100644 (file)
@@ -28,7 +28,9 @@ import org.sonar.db.DbSession;
 import org.sonar.db.alm.setting.AlmSettingDto;
 import org.sonar.server.user.UserSession;
 
+import static org.apache.commons.lang.StringUtils.isBlank;
 import static org.apache.commons.lang.StringUtils.isNotBlank;
+import static org.apache.commons.lang.StringUtils.removeEnd;
 
 public class UpdateGithubAction implements AlmSettingsWsAction {
 
@@ -39,6 +41,7 @@ public class UpdateGithubAction implements AlmSettingsWsAction {
   private static final String PARAM_CLIENT_ID = "clientId";
   private static final String PARAM_CLIENT_SECRET = "clientSecret";
   private static final String PARAM_PRIVATE_KEY = "privateKey";
+  private static final String PARAM_WEBHOOK_SECRET = "webhookSecret";
 
   private final DbClient dbClient;
   private final UserSession userSession;
@@ -57,7 +60,9 @@ public class UpdateGithubAction implements AlmSettingsWsAction {
         "Requires the 'Administer System' permission")
       .setPost(true)
       .setSince("8.1")
-      .setChangelog(new Change("8.7", String.format("Parameter '%s' is no longer required", PARAM_PRIVATE_KEY)),
+      .setChangelog(
+        new Change("9.7", String.format("Optional parameter '%s' was added", PARAM_WEBHOOK_SECRET)),
+        new Change("8.7", String.format("Parameter '%s' is no longer required", PARAM_PRIVATE_KEY)),
         new Change("8.7", String.format("Parameter '%s' is no longer required", PARAM_CLIENT_SECRET)))
       .setHandler(this);
 
@@ -89,50 +94,59 @@ public class UpdateGithubAction implements AlmSettingsWsAction {
       .setRequired(false)
       .setMaximumLength(160)
       .setDescription("GitHub App Client Secret");
+    action.createParam(PARAM_WEBHOOK_SECRET)
+      .setRequired(false)
+      .setMaximumLength(160)
+      .setDescription("GitHub App Webhook Secret");
   }
 
   @Override
   public void handle(Request request, Response response) throws Exception {
     userSession.checkIsSystemAdministrator();
-    doHandle(request);
+    tryDoHandle(request);
     response.noContent();
   }
 
-  private void doHandle(Request request) {
+  private void tryDoHandle(Request request) {
+    try (DbSession dbSession = dbClient.openSession(false)) {
+      doHandle(request, dbSession);
+    }
+  }
+
+  private void doHandle(Request request, DbSession dbSession) {
     String key = request.mandatoryParam(PARAM_KEY);
     String newKey = request.param(PARAM_NEW_KEY);
-    String url = request.mandatoryParam(PARAM_URL);
-    String appId = request.mandatoryParam(PARAM_APP_ID);
-    String clientId = request.mandatoryParam(PARAM_CLIENT_ID);
-    String clientSecret = request.param(PARAM_CLIENT_SECRET);
+    if (isNotBlank(newKey) && !newKey.equals(key)) {
+      almSettingsSupport.checkAlmSettingDoesNotAlreadyExist(dbSession, newKey);
+    }
+
+    AlmSettingDto almSettingDto = almSettingsSupport.getAlmSetting(dbSession, key);
+
     String privateKey = request.param(PARAM_PRIVATE_KEY);
+    if (isNotBlank(privateKey)) {
+      almSettingDto.setPrivateKey(privateKey);
+    }
 
-    if (url.endsWith("/")) {
-      url = url.substring(0, url.length() - 1);
+    String clientSecret = request.param(PARAM_CLIENT_SECRET);
+    if (isNotBlank(clientSecret)) {
+      almSettingDto.setClientSecret(clientSecret);
     }
 
-    try (DbSession dbSession = dbClient.openSession(false)) {
-      AlmSettingDto almSettingDto = almSettingsSupport.getAlmSetting(dbSession, key);
-      if (isNotBlank(newKey) && !newKey.equals(key)) {
-        almSettingsSupport.checkAlmSettingDoesNotAlreadyExist(dbSession, newKey);
-      }
-
-      if (isNotBlank(privateKey)) {
-        almSettingDto.setPrivateKey(privateKey);
-      }
-
-      if (isNotBlank(clientSecret)) {
-        almSettingDto.setClientSecret(clientSecret);
-      }
-
-      dbClient.almSettingDao().update(dbSession, almSettingDto
-          .setKey(isNotBlank(newKey) ? newKey : key)
-          .setUrl(url)
-          .setAppId(appId)
-          .setClientId(clientId),
-        clientSecret != null || privateKey != null);
-      dbSession.commit();
+    boolean hasWebhookSecretParam = request.hasParam(PARAM_WEBHOOK_SECRET);
+    if (hasWebhookSecretParam) {
+      String webhookSecret = request.getParam(PARAM_WEBHOOK_SECRET).getValue();
+      almSettingDto.setWebhookSecret(isBlank(webhookSecret) ? null : webhookSecret);
     }
+
+    almSettingDto
+      .setKey(isNotBlank(newKey) ? newKey : key)
+      .setUrl(removeEnd(request.mandatoryParam(PARAM_URL), "/"))
+      .setAppId(request.mandatoryParam(PARAM_APP_ID))
+      .setClientId(request.mandatoryParam(PARAM_CLIENT_ID));
+
+    boolean isAnySecretUpdated = clientSecret != null || privateKey != null || hasWebhookSecretParam;
+    dbClient.almSettingDao().update(dbSession, almSettingDto, isAnySecretUpdated);
+    dbSession.commit();
   }
 
 }
index 3aaa03b45c04caa66cd95aa2104cfc7102008cb0..5d3b478838dd99041dcf973fbf8cd0af363a8e8b 100644 (file)
  */
 package org.sonar.server.almsettings.ws;
 
+import org.assertj.core.groups.Tuple;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.api.config.internal.Encryption;
+import org.sonar.api.resources.ResourceTypes;
+import org.sonar.api.server.ws.Change;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.db.DbTester;
+import org.sonar.db.alm.setting.ALM;
 import org.sonar.db.alm.setting.AlmSettingDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.server.almsettings.MultipleAlmFeatureProvider;
@@ -43,6 +47,8 @@ import static org.mockito.Mockito.when;
 
 public class CreateGithubActionTest {
 
+  private static final String APP_ID = "12345";
+
   @Rule
   public UserSessionRule userSession = UserSessionRule.standalone();
   @Rule
@@ -51,71 +57,75 @@ public class CreateGithubActionTest {
   private final Encryption encryption = mock(Encryption.class);
   private final MultipleAlmFeatureProvider multipleAlmFeatureProvider = mock(MultipleAlmFeatureProvider.class);
 
-  private WsActionTester ws = new WsActionTester(new CreateGithubAction(db.getDbClient(), userSession,
-    new AlmSettingsSupport(db.getDbClient(), userSession, new ComponentFinder(db.getDbClient(), null),
+  private final WsActionTester ws = new WsActionTester(new CreateGithubAction(db.getDbClient(), userSession,
+    new AlmSettingsSupport(db.getDbClient(), userSession, new ComponentFinder(db.getDbClient(), mock(ResourceTypes.class)),
       multipleAlmFeatureProvider)));
 
   @Before
-  public void before() {
+  public void setUp() {
     when(multipleAlmFeatureProvider.enabled()).thenReturn(false);
+    UserDto user = db.users().insertUser();
+    userSession.logIn(user).setSystemAdministrator();
   }
 
   @Test
   public void create() {
-    UserDto user = db.users().insertUser();
-    userSession.logIn(user).setSystemAdministrator();
+    buildTestRequest().execute();
 
-    ws.newRequest()
+    assertThat(db.getDbClient().almSettingDao().selectAll(db.getSession()))
+      .extracting(AlmSettingDto::getKey, AlmSettingDto::getUrl, AlmSettingDto::getAppId,
+        s -> s.getDecryptedPrivateKey(encryption), AlmSettingDto::getClientId, s -> s.getDecryptedClientSecret(encryption), s -> s.getDecryptedWebhookSecret(encryption))
+      .containsOnly(tuple("GitHub Server - Dev Team", "https://github.enterprise.com", APP_ID, "678910", "client_1234", "client_so_secret", null));
+  }
+
+  private TestRequest buildTestRequest() {
+    return ws.newRequest()
       .setParam("key", "GitHub Server - Dev Team")
       .setParam("url", "https://github.enterprise.com")
-      .setParam("appId", "12345")
+      .setParam("appId", APP_ID)
       .setParam("privateKey", "678910")
       .setParam("clientId", "client_1234")
-      .setParam("clientSecret", "client_so_secret")
-      .execute();
+      .setParam("clientSecret", "client_so_secret");
+  }
+
+  @Test
+  public void create_withWebhookSecret() {
+    buildTestRequest().setParam("webhookSecret", "webhook_secret").execute();
 
     assertThat(db.getDbClient().almSettingDao().selectAll(db.getSession()))
       .extracting(AlmSettingDto::getKey, AlmSettingDto::getUrl, AlmSettingDto::getAppId,
-        s -> s.getDecryptedPrivateKey(encryption), AlmSettingDto::getClientId, s -> s.getDecryptedClientSecret(encryption))
-      .containsOnly(tuple("GitHub Server - Dev Team", "https://github.enterprise.com", "12345", "678910", "client_1234", "client_so_secret"));
+        s -> s.getDecryptedPrivateKey(encryption), AlmSettingDto::getClientId, s -> s.getDecryptedClientSecret(encryption), s -> s.getDecryptedWebhookSecret(encryption))
+      .containsOnly(tuple("GitHub Server - Dev Team", "https://github.enterprise.com", "12345", "678910", "client_1234", "client_so_secret", "webhook_secret"));
+  }
+
+  @Test
+  public void create_withEmptyWebhookSecret_shouldNotPersist() {
+    buildTestRequest().setParam("webhookSecret", "").execute();
+
+    assertThat(db.getDbClient().almSettingDao().selectByAlmAndAppId(db.getSession(), ALM.GITHUB, APP_ID))
+      .map(almSettingDto -> almSettingDto.getDecryptedWebhookSecret(encryption))
+      .isEmpty();
   }
 
   @Test
   public void remove_trailing_slash() {
-    UserDto user = db.users().insertUser();
-    userSession.logIn(user).setSystemAdministrator();
 
-    ws.newRequest()
-      .setParam("key", "GitHub Server - Dev Team")
-      .setParam("url", "https://github.enterprise.com/")
-      .setParam("appId", "12345")
-      .setParam("privateKey", "678910")
-      .setParam("clientId", "client_1234")
-      .setParam("clientSecret", "client_so_secret")
-      .execute();
+    buildTestRequest().setParam("url", "https://github.enterprise.com/").execute();
 
     assertThat(db.getDbClient().almSettingDao().selectAll(db.getSession()))
       .extracting(AlmSettingDto::getKey, AlmSettingDto::getUrl, AlmSettingDto::getAppId,
         s -> s.getDecryptedPrivateKey(encryption), AlmSettingDto::getClientId, s -> s.getDecryptedClientSecret(encryption))
-      .containsOnly(tuple("GitHub Server - Dev Team", "https://github.enterprise.com", "12345", "678910", "client_1234", "client_so_secret"));
+      .containsOnly(tuple("GitHub Server - Dev Team", "https://github.enterprise.com", APP_ID, "678910", "client_1234", "client_so_secret"));
   }
 
   @Test
   public void fail_when_key_is_already_used() {
     when(multipleAlmFeatureProvider.enabled()).thenReturn(true);
-    UserDto user = db.users().insertUser();
-    userSession.logIn(user).setSystemAdministrator();
     AlmSettingDto gitHubAlmSetting = db.almSettings().insertGitHubAlmSetting();
 
-    TestRequest request = ws.newRequest()
-      .setParam("key", gitHubAlmSetting.getKey())
-      .setParam("url", "https://github.enterprise.com")
-      .setParam("appId", "12345")
-      .setParam("privateKey", "678910")
-      .setParam("clientId", "client_1234")
-      .setParam("clientSecret", "client_so_secret");
+    TestRequest request = buildTestRequest().setParam("key", gitHubAlmSetting.getKey());
 
-    assertThatThrownBy(() -> request.execute())
+    assertThatThrownBy(request::execute)
       .isInstanceOf(IllegalArgumentException.class)
       .hasMessageContaining(String.format("An ALM setting with key '%s' already exist", gitHubAlmSetting.getKey()));
   }
@@ -123,19 +133,11 @@ public class CreateGithubActionTest {
   @Test
   public void fail_when_no_multiple_instance_allowed() {
     when(multipleAlmFeatureProvider.enabled()).thenReturn(false);
-    UserDto user = db.users().insertUser();
-    userSession.logIn(user).setSystemAdministrator();
     db.almSettings().insertGitHubAlmSetting();
 
-    TestRequest request = ws.newRequest()
-      .setParam("key", "key")
-      .setParam("url", "https://github.enterprise.com")
-      .setParam("appId", "12345")
-      .setParam("privateKey", "678910")
-      .setParam("clientId", "client_1234")
-      .setParam("clientSecret", "client_so_secret");
+    TestRequest request = buildTestRequest();
 
-    assertThatThrownBy(() -> request.execute())
+    assertThatThrownBy(request::execute)
       .isInstanceOf(BadRequestException.class)
       .hasMessageContaining("A GITHUB setting is already defined");
   }
@@ -145,15 +147,9 @@ public class CreateGithubActionTest {
     UserDto user = db.users().insertUser();
     userSession.logIn(user);
 
-    TestRequest request = ws.newRequest()
-      .setParam("key", "GitHub Server - Dev Team")
-      .setParam("url", "https://github.enterprise.com")
-      .setParam("appId", "12345")
-      .setParam("privateKey", "678910")
-      .setParam("clientId", "client_1234")
-      .setParam("clientSecret", "client_so_secret");
+    TestRequest request = buildTestRequest();
 
-    assertThatThrownBy(() -> request.execute())
+    assertThatThrownBy(request::execute)
       .isInstanceOf(ForbiddenException.class);
   }
 
@@ -165,6 +161,20 @@ public class CreateGithubActionTest {
     assertThat(def.isPost()).isTrue();
     assertThat(def.params())
       .extracting(WebService.Param::key, WebService.Param::isRequired)
-      .containsExactlyInAnyOrder(tuple("key", true), tuple("url", true), tuple("appId", true), tuple("privateKey", true), tuple("clientId", true), tuple("clientSecret", true));
+      .containsExactlyInAnyOrder(
+        tuple("key", true),
+        tuple("url", true),
+        tuple("appId", true),
+        tuple("privateKey", true),
+        tuple("clientId", true),
+        tuple("clientSecret", true),
+        tuple("webhookSecret", false));
+  }
+
+  @Test
+  public void definition_shouldHaveChangeLog() {
+    assertThat(ws.getDef().changelog()).extracting(Change::getVersion, Change::getDescription).containsExactly(
+      new Tuple("9.7", "Optional parameter 'webhookSecret' was added")
+    );
   }
 }
index d6957f3404d4eee90171f0c13d24e644457089ac..1546a24be1866124711be32d9e194d2735035bfd 100644 (file)
  */
 package org.sonar.server.almsettings.ws;
 
+import com.tngtech.java.junit.dataprovider.DataProvider;
+import com.tngtech.java.junit.dataprovider.DataProviderRunner;
+import com.tngtech.java.junit.dataprovider.UseDataProvider;
+import org.assertj.core.groups.Tuple;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
 import org.sonar.api.config.internal.Encryption;
+import org.sonar.api.resources.ResourceTypes;
+import org.sonar.api.server.ws.Change;
 import org.sonar.api.server.ws.WebService;
+import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.alm.setting.AlmSettingDto;
+import org.sonar.db.audit.AuditPersister;
+import org.sonar.db.audit.model.DevOpsPlatformSettingNewValue;
+import org.sonar.db.audit.model.SecretNewValue;
 import org.sonar.db.user.UserDto;
 import org.sonar.server.almsettings.MultipleAlmFeatureProvider;
 import org.sonar.server.component.ComponentFinder;
@@ -35,38 +48,43 @@ import org.sonar.server.ws.TestRequest;
 import org.sonar.server.ws.WsActionTester;
 
 import static java.lang.String.format;
+import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.groups.Tuple.tuple;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.sonar.db.alm.setting.ALM.GITHUB;
 
+@RunWith(DataProviderRunner.class)
 public class UpdateGithubActionTest {
 
+  private final AuditPersister auditPersister = mock(AuditPersister.class);
+
   @Rule
   public UserSessionRule userSession = UserSessionRule.standalone();
   @Rule
-  public DbTester db = DbTester.create();
+  public DbTester db = DbTester.create(auditPersister);
 
   private final Encryption encryption = mock(Encryption.class);
 
-  private WsActionTester ws = new WsActionTester(new UpdateGithubAction(db.getDbClient(), userSession,
-    new AlmSettingsSupport(db.getDbClient(), userSession, new ComponentFinder(db.getDbClient(), null),
+  private final WsActionTester ws = new WsActionTester(new UpdateGithubAction(db.getDbClient(), userSession,
+    new AlmSettingsSupport(db.getDbClient(), userSession, new ComponentFinder(db.getDbClient(), mock(ResourceTypes.class)),
       mock(MultipleAlmFeatureProvider.class))));
 
-  @Test
-  public void update() {
+  private AlmSettingDto almSettingDto;
+
+  @Before
+  public void setUp() {
+    almSettingDto = db.almSettings().insertGitHubAlmSetting();
     UserDto user = db.users().insertUser();
     userSession.logIn(user).setSystemAdministrator();
-    AlmSettingDto almSettingDto = db.almSettings().insertGitHubAlmSetting();
+  }
 
-    ws.newRequest()
-      .setParam("key", almSettingDto.getKey())
-      .setParam("url", "https://github.enterprise-unicorn.com")
-      .setParam("appId", "54321")
-      .setParam("privateKey", "10987654321")
-      .setParam("clientId", "client_1234")
-      .setParam("clientSecret", "client_so_secret")
-      .execute();
+  @Test
+  public void update() {
+    buildTestRequest().execute();
 
     assertThat(db.getDbClient().almSettingDao().selectAll(db.getSession()))
       .extracting(AlmSettingDto::getKey, AlmSettingDto::getUrl, AlmSettingDto::getAppId,
@@ -74,20 +92,19 @@ public class UpdateGithubActionTest {
       .containsOnly(tuple(almSettingDto.getKey(), "https://github.enterprise-unicorn.com", "54321", "10987654321", "client_1234", "client_so_secret"));
   }
 
-  @Test
-  public void update_url_with_trailing_slash() {
-    UserDto user = db.users().insertUser();
-    userSession.logIn(user).setSystemAdministrator();
-    AlmSettingDto almSettingDto = db.almSettings().insertGitHubAlmSetting();
-
-    ws.newRequest()
+  private TestRequest buildTestRequest() {
+    return ws.newRequest()
       .setParam("key", almSettingDto.getKey())
-      .setParam("url", "https://github.enterprise-unicorn.com/")
+      .setParam("url", "https://github.enterprise-unicorn.com")
       .setParam("appId", "54321")
       .setParam("privateKey", "10987654321")
       .setParam("clientId", "client_1234")
-      .setParam("clientSecret", "client_so_secret")
-      .execute();
+      .setParam("clientSecret", "client_so_secret");
+  }
+
+  @Test
+  public void update_url_with_trailing_slash() {
+    buildTestRequest().setParam("url", "https://github.enterprise-unicorn.com/").execute();
 
     assertThat(db.getDbClient().almSettingDao().selectAll(db.getSession()))
       .extracting(AlmSettingDto::getKey, AlmSettingDto::getUrl, AlmSettingDto::getAppId,
@@ -97,19 +114,7 @@ public class UpdateGithubActionTest {
 
   @Test
   public void update_with_new_key() {
-    UserDto user = db.users().insertUser();
-    userSession.logIn(user).setSystemAdministrator();
-    AlmSettingDto almSettingDto = db.almSettings().insertGitHubAlmSetting();
-
-    ws.newRequest()
-      .setParam("key", almSettingDto.getKey())
-      .setParam("newKey", "GitHub Server - Infra Team")
-      .setParam("url", "https://github.enterprise-unicorn.com")
-      .setParam("appId", "54321")
-      .setParam("privateKey", "10987654321")
-      .setParam("clientId", "client_1234")
-      .setParam("clientSecret", "client_so_secret")
-      .execute();
+    buildTestRequest().setParam("newKey", "GitHub Server - Infra Team").execute();
 
     assertThat(db.getDbClient().almSettingDao().selectAll(db.getSession()))
       .extracting(AlmSettingDto::getKey, AlmSettingDto::getUrl, AlmSettingDto::getAppId,
@@ -119,16 +124,7 @@ public class UpdateGithubActionTest {
 
   @Test
   public void update_without_private_key_nor_client_secret() {
-    UserDto user = db.users().insertUser();
-    userSession.logIn(user).setSystemAdministrator();
-    AlmSettingDto almSettingDto = db.almSettings().insertGitHubAlmSetting();
-
-    ws.newRequest()
-      .setParam("key", almSettingDto.getKey())
-      .setParam("url", "https://github.enterprise-unicorn.com/")
-      .setParam("appId", "54321")
-      .setParam("clientId", "client_1234")
-      .execute();
+    buildTestRequestWithoutSecrets().execute();
 
     assertThat(db.getDbClient().almSettingDao().selectAll(db.getSession()))
       .extracting(AlmSettingDto::getKey, AlmSettingDto::getUrl, AlmSettingDto::getAppId,
@@ -137,42 +133,35 @@ public class UpdateGithubActionTest {
         almSettingDto.getDecryptedPrivateKey(encryption), "client_1234", almSettingDto.getDecryptedClientSecret(encryption)));
   }
 
+
+  private TestRequest buildTestRequestWithoutSecrets() {
+    return ws.newRequest()
+      .setParam("key", almSettingDto.getKey())
+      .setParam("url", "https://github.enterprise-unicorn.com/")
+      .setParam("appId", "54321")
+      .setParam("clientId", "client_1234");
+  }
+
   @Test
   public void fail_when_key_does_not_match_existing_alm_setting() {
-    UserDto user = db.users().insertUser();
-    userSession.logIn(user).setSystemAdministrator();
-
-    TestRequest request = ws.newRequest()
+    TestRequest request = buildTestRequest()
       .setParam("key", "unknown")
-      .setParam("newKey", "GitHub Server - Infra Team")
-      .setParam("url", "https://github.enterprise-unicorn.com")
-      .setParam("appId", "54321")
-      .setParam("privateKey", "10987654321")
-      .setParam("clientId", "client_1234")
-      .setParam("clientSecret", "client_so_secret");
+      .setParam("newKey", "GitHub Server - Infra Team");
 
-    assertThatThrownBy(() -> request.execute())
+    assertThatThrownBy(request::execute)
       .isInstanceOf(NotFoundException.class)
       .hasMessageContaining("ALM setting with key 'unknown' cannot be found");
   }
 
   @Test
   public void fail_when_new_key_matches_existing_alm_setting() {
-    UserDto user = db.users().insertUser();
-    userSession.logIn(user).setSystemAdministrator();
-    AlmSettingDto almSetting1 = db.almSettings().insertGitHubAlmSetting();
     AlmSettingDto almSetting2 = db.almSettings().insertGitHubAlmSetting();
 
-    TestRequest request = ws.newRequest()
-      .setParam("key", almSetting1.getKey())
-      .setParam("newKey", almSetting2.getKey())
-      .setParam("url", "https://github.enterprise-unicorn.com")
-      .setParam("appId", "54321")
-      .setParam("privateKey", "10987654321")
-      .setParam("clientId", "client_1234")
-      .setParam("clientSecret", "client_so_secret");
+    TestRequest request = buildTestRequest()
+      .setParam("key", almSettingDto.getKey())
+      .setParam("newKey", almSetting2.getKey());
 
-    assertThatThrownBy(() -> request.execute())
+    assertThatThrownBy(request::execute)
       .isInstanceOf(IllegalArgumentException.class)
       .hasMessageContaining(format("An ALM setting with key '%s' already exists", almSetting2.getKey()));
   }
@@ -181,19 +170,10 @@ public class UpdateGithubActionTest {
   public void fail_when_missing_administer_system_permission() {
     UserDto user = db.users().insertUser();
     userSession.logIn(user);
-    AlmSettingDto almSettingDto = db.almSettings().insertGitHubAlmSetting();
 
-    TestRequest request = ws.newRequest()
-      .setParam("key", almSettingDto.getKey())
-      .setParam("newKey", "GitHub Server - Infra Team")
-      .setParam("url", "https://github.enterprise-unicorn.com")
-      .setParam("appId", "54321")
-      .setParam("privateKey", "10987654321")
-      .setParam("clientId", "client_1234")
-      .setParam("clientSecret", "client_so_secret");
+    TestRequest request = buildTestRequest();
 
-    assertThatThrownBy(() -> request.execute())
-      .isInstanceOf(ForbiddenException.class);
+    assertThatThrownBy(request::execute).isInstanceOf(ForbiddenException.class);
   }
 
   @Test
@@ -204,8 +184,85 @@ public class UpdateGithubActionTest {
     assertThat(def.isPost()).isTrue();
     assertThat(def.params())
       .extracting(WebService.Param::key, WebService.Param::isRequired)
-      .containsExactlyInAnyOrder(tuple("key", true), tuple("newKey", false), tuple("url", true), tuple("appId", true), tuple("privateKey", false), tuple("clientId", true),
-        tuple("clientSecret", false));
+      .containsExactlyInAnyOrder(
+        tuple("key", true),
+        tuple("newKey", false),
+        tuple("url", true),
+        tuple("appId", true),
+        tuple("privateKey", false),
+        tuple("clientId", true),
+        tuple("clientSecret", false),
+        tuple("webhookSecret", false));
+  }
+
+  @Test
+  public void update_withWebhookSecret() {
+    buildTestRequest().setParam("webhookSecret", "webhook_secret").execute();
+
+    assertThat(db.getDbClient().almSettingDao().selectAll(db.getSession()))
+      .extracting(almSettings -> almSettings.getDecryptedWebhookSecret(encryption))
+      .containsOnly("webhook_secret");
+  }
+
+  @Test
+  public void update_withoutWebhookSecret_shouldNotOverrideExistingValue() {
+    buildTestRequest().setParam("webhookSecret", "webhook_secret").execute();
+
+    buildTestRequest().execute();
+
+    assertThat(db.getDbClient().almSettingDao().selectAll(db.getSession()))
+      .extracting(almSettings -> almSettings.getDecryptedWebhookSecret(encryption))
+      .containsOnly("webhook_secret");
+  }
+
+  @Test
+  public void update_withEmptyValue_shouldResetWebhookSecret() {
+    buildTestRequest().setParam("webhookSecret", "webhook_secret").execute();
+
+    buildTestRequest().setParam("webhookSecret", "").execute();
+
+    assertThat(db.getDbClient().almSettingDao().selectAll(db.getSession()))
+      .extracting(almSettings -> almSettings.getDecryptedWebhookSecret(encryption))
+      .containsOnly((String) null);
+  }
+
+  @Test
+  public void definition_shouldHaveChangeLog() {
+    assertThat(ws.getDef().changelog()).extracting(Change::getVersion, Change::getDescription).containsExactly(
+      new Tuple("9.7", "Optional parameter 'webhookSecret' was added"),
+      new Tuple("8.7", "Parameter 'privateKey' is no longer required"),
+      new Tuple("8.7", "Parameter 'clientSecret' is no longer required")
+    );
+  }
+
+  @Test
+  @UseDataProvider("secretParams")
+  public void update_withSecretChange_shouldAuditDevOpsPlatformSecret(String secretParam) {
+    buildTestRequestWithoutSecrets().setParam(secretParam, randomAlphanumeric(10)).execute();
+    SecretNewValue expected = new SecretNewValue("DevOpsPlatform", GITHUB.getId());
+    ArgumentCaptor<SecretNewValue> captor = ArgumentCaptor.forClass(SecretNewValue.class);
+
+    verify(auditPersister).updateDevOpsPlatformSecret(any(DbSession.class), captor.capture());
+    assertThat(captor.getValue()).usingRecursiveComparison().isEqualTo(expected);
+  }
+
+  @DataProvider
+  public static Object[][] secretParams() {
+    return new Object[][] {
+      {"webhookSecret"},
+      {"clientSecret"},
+      {"privateKey"}
+    };
+  }
+
+  @Test
+  public void update_withNoSecretChanges_shouldAuditDevOpsPlatformSettings() {
+    buildTestRequestWithoutSecrets().execute();
+    DevOpsPlatformSettingNewValue expected = new DevOpsPlatformSettingNewValue(almSettingDto.getUuid(), almSettingDto.getKey());
+    ArgumentCaptor<DevOpsPlatformSettingNewValue> captor = ArgumentCaptor.forClass(DevOpsPlatformSettingNewValue.class);
+
+    verify(auditPersister).updateDevOpsPlatformSetting(any(DbSession.class), captor.capture());
+    assertThat(captor.getValue()).usingRecursiveComparison().comparingOnlyFields("devOpsPlatformSettingUuid", "key").isEqualTo(expected);
   }
 
 }