SONAR-15287 When updating a secret, audit logs should log the action

7 files changed, 91 insertions, 8 deletions

diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDao.java
index 973e7e82f44..e623b6407eb 100644
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDao.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDao.java
@@ -27,6 +27,7 @@ import org.sonar.db.Dao;
 import org.sonar.db.DbSession;
 import org.sonar.db.audit.AuditPersister;
 import org.sonar.db.audit.model.DevOpsPlatformSettingNewValue;
+import org.sonar.db.audit.model.SecretNewValue;
 
 public class AlmSettingDao implements Dao {
 
@@ -86,12 +87,15 @@ public class AlmSettingDao implements Dao {
     }
   }
 
-  public void update(DbSession dbSession, AlmSettingDto almSettingDto) {
+  public void update(DbSession dbSession, AlmSettingDto almSettingDto, boolean updateSecret) {
     long now = system2.now();
     almSettingDto.setUpdatedAt(now);
     getMapper(dbSession).update(almSettingDto);
 
     if (auditPersister != null) {
+      if (updateSecret) {
+        auditPersister.updateDevOpsPlatformSecret(dbSession, new SecretNewValue("DevOpsPlatform", almSettingDto.getRawAlm()));
+      }
       auditPersister.updateDevOpsPlatformSetting(dbSession, new DevOpsPlatformSettingNewValue(almSettingDto));
     }
   }
diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/audit/AuditPersister.java b/server/sonar-db-dao/src/main/java/org/sonar/db/audit/AuditPersister.java
index 4a393c6ea76..ef4b852a6f5 100644
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/audit/AuditPersister.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/audit/AuditPersister.java
@@ -38,6 +38,12 @@ public interface AuditPersister {
 
   void updateUser(DbSession dbSession, NewValue newValue);
 
+  void updateUserPassword(DbSession dbSession, NewValue newValue);
+
+  void updateWebhookSecret(DbSession dbSession, NewValue newValue);
+
+  void updateDevOpsPlatformSecret(DbSession dbSession, NewValue newValue);
+
   void deactivateUser(DbSession dbSession, NewValue newValue);
 
   void addUserToGroup(DbSession dbSession, NewValue newValue);
diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/SecretNewValue.java b/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/SecretNewValue.java
new file mode 100644
index 00000000000..85e47a3d8f6
--- /dev/null
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/SecretNewValue.java
@@ -0,0 +1,35 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2021 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.
+ */
+package org.sonar.db.audit.model;
+
+public class SecretNewValue extends NewValue {
+  private String targetKey;
+  private String targetValue;
+
+  public SecretNewValue(String targetKey, String targetValue) {
+    this.targetKey = targetKey;
+    this.targetValue = targetValue;
+  }
+
+  @Override
+  public String toString() {
+    return String.format("{\"%s\":\"%s\"}", targetKey, targetValue);
+  }
+}
diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDao.java
index 43e962d7f97..38c6be76329 100644
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDao.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDao.java
@@ -26,6 +26,7 @@ import org.sonar.api.utils.System2;
 import org.sonar.db.Dao;
 import org.sonar.db.DbSession;
 import org.sonar.db.audit.AuditPersister;
+import org.sonar.db.audit.model.SecretNewValue;
 import org.sonar.db.audit.model.WebhookNewValue;
 import org.sonar.db.project.ProjectDto;
 
@@ -34,7 +35,6 @@ public class WebhookDao implements Dao {
   private final System2 system2;
   private AuditPersister auditPersister;
 
-
   public WebhookDao(System2 system2) {
     this.system2 = system2;
   }
@@ -68,6 +68,9 @@ public class WebhookDao implements Dao {
     mapper(dbSession).update(dto.setUpdatedAt(system2.now()));
 
     if (auditPersister != null) {
+      if (dto.getSecret() != null) {
+        auditPersister.updateWebhookSecret(dbSession, new SecretNewValue("webhook_name", dto.getName()));
+      }
       auditPersister.updateWebhook(dbSession, new WebhookNewValue(dto, projectKey, projectName));
     }
   }
diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoTest.java
index b639e027b91..b2b3a20797b 100644
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoTest.java
@@ -121,7 +121,7 @@ public class AlmSettingDaoTest {
     almSettingDto.setKey("updated key");
 
     system2.setNow(NOW + 1);
-    underTest.update(dbSession, almSettingDto);
+    underTest.update(dbSession, almSettingDto, false);
 
     AlmSettingDto result = underTest.selectByUuid(dbSession, A_UUID).get();
     assertThat(result)
diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoWithPersisterTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoWithPersisterTest.java
index 9d734652e28..c88d3f50dfd 100644
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoWithPersisterTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoWithPersisterTest.java
@@ -28,6 +28,7 @@ import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.audit.AuditPersister;
 import org.sonar.db.audit.model.DevOpsPlatformSettingNewValue;
+import org.sonar.db.audit.model.SecretNewValue;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.eq;
@@ -54,6 +55,7 @@ public class AlmSettingDaoWithPersisterTest {
 
   @Test
   public void insertAndUpdateArePersisted() {
+    ArgumentCaptor<SecretNewValue> secretNewValueCaptor = ArgumentCaptor.forClass(SecretNewValue.class);
     when(uuidFactory.create()).thenReturn(A_UUID);
     AlmSettingDto almSettingDto = newGithubAlmSettingDto()
       .setKey("key")
@@ -68,7 +70,8 @@ public class AlmSettingDaoWithPersisterTest {
     assertThat(newValue)
       .extracting("devOpsPlatformSettingUuid", "key")
       .containsExactly(almSettingDto.getUuid(), almSettingDto.getKey());
-    assertThat(newValue).hasToString("{\"devOpsPlatformSettingUuid\": \"1\", \"key\": \"key\", \"devOpsPlatformName\": \"id1\", \"url\": \"url\", \"appId\": \"id1\", \"clientId\": \"cid1\" }");
+    assertThat(newValue)
+      .hasToString("{\"devOpsPlatformSettingUuid\": \"1\", \"key\": \"key\", \"devOpsPlatformName\": \"id1\", \"url\": \"url\", \"appId\": \"id1\", \"clientId\": \"cid1\" }");
 
     almSettingDto.setPrivateKey("updated private key");
     almSettingDto.setAppId("updated app id");
@@ -76,15 +79,19 @@ public class AlmSettingDaoWithPersisterTest {
     almSettingDto.setPersonalAccessToken("updated pat");
     almSettingDto.setKey("updated key");
 
-    underTest.update(dbSession, almSettingDto);
+    underTest.update(dbSession, almSettingDto, true);
+
+    verify(auditPersister).updateDevOpsPlatformSecret(eq(dbSession), secretNewValueCaptor.capture());
+    SecretNewValue secretNewValue = secretNewValueCaptor.getValue();
+    assertThat(secretNewValue).hasToString(String.format("{\"DevOpsPlatform\":\"%s\"}", almSettingDto.getRawAlm()));
 
     verify(auditPersister).updateDevOpsPlatformSetting(eq(dbSession), newValueCaptor.capture());
     newValue = newValueCaptor.getValue();
     assertThat(newValue)
-      .extracting("devOpsPlatformSettingUuid", "key","appId", "devOpsPlatformName", "url", "clientId")
+      .extracting("devOpsPlatformSettingUuid", "key", "appId", "devOpsPlatformName", "url", "clientId")
       .containsExactly(almSettingDto.getUuid(), almSettingDto.getKey(), almSettingDto.getAppId(), almSettingDto.getAppId(), almSettingDto.getUrl(), almSettingDto.getClientId());
     assertThat(newValue).hasToString("{\"devOpsPlatformSettingUuid\": \"1\", \"key\": \"updated key\", \"devOpsPlatformName\": \"updated app id\", "
-        + "\"url\": \"updated url\", \"appId\": \"updated app id\", \"clientId\": \"cid1\" }");
+      + "\"url\": \"updated url\", \"appId\": \"updated app id\", \"clientId\": \"cid1\" }");
   }
 
   @Test
diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoWithPersisterTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoWithPersisterTest.java
index 47b0f29498d..cb3429d319f 100644
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoWithPersisterTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoWithPersisterTest.java
@@ -27,6 +27,7 @@ import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.audit.AuditPersister;
+import org.sonar.db.audit.model.SecretNewValue;
 import org.sonar.db.audit.model.WebhookNewValue;
 import org.sonar.db.component.ComponentDbTester;
 import org.sonar.db.project.ProjectDto;
@@ -35,6 +36,8 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 
@@ -51,6 +54,7 @@ public class WebhookDaoWithPersisterTest {
   private final ComponentDbTester componentDbTester = dbTester.components();
 
   private final ArgumentCaptor<WebhookNewValue> newValueCaptor = ArgumentCaptor.forClass(WebhookNewValue.class);
+  private final ArgumentCaptor<SecretNewValue> secretNewValueCaptor = ArgumentCaptor.forClass(SecretNewValue.class);
 
   @Test
   public void insertGlobalWebhookIsPersisted() {
@@ -92,7 +96,7 @@ public class WebhookDaoWithPersisterTest {
   }
 
   @Test
-  public void updateGlobalWebhookIsPersisted() {
+  public void updateGlobalWebhookIsPersistedWithoutSecret() {
     WebhookDto dto = webhookDbTester.insertGlobalWebhook();
     dto = dto
       .setName("a-fancy-webhook")
@@ -101,6 +105,8 @@ public class WebhookDaoWithPersisterTest {
 
     underTest.update(dbSession, dto, null, null);
 
+    verify(auditPersister, never()).updateWebhookSecret(eq(dbSession), any());
+
     verify(auditPersister).updateWebhook(eq(dbSession), newValueCaptor.capture());
     WebhookNewValue newValue = newValueCaptor.getValue();
     assertThat(newValue)
@@ -110,6 +116,28 @@ public class WebhookDaoWithPersisterTest {
   }
 
   @Test
+  public void updateGlobalWebhookIsPersistedWithSecret() {
+    WebhookDto dto = webhookDbTester.insertGlobalWebhook();
+    dto = dto
+            .setName("a-fancy-webhook")
+            .setUrl("http://www.fancy-webhook.io")
+            .setSecret("new secret");
+
+    underTest.update(dbSession, dto, null, null);
+
+    verify(auditPersister).updateWebhookSecret(eq(dbSession), secretNewValueCaptor.capture());
+    SecretNewValue secretNewValue = secretNewValueCaptor.getValue();
+    assertThat(secretNewValue).hasToString(String.format("{\"webhook_name\":\"%s\"}", dto.getName()));
+
+    verify(auditPersister).updateWebhook(eq(dbSession), newValueCaptor.capture());
+    WebhookNewValue newValue = newValueCaptor.getValue();
+    assertThat(newValue)
+            .extracting(WebhookNewValue::getWebhookUuid, WebhookNewValue::getName, WebhookNewValue::getUrl)
+            .containsExactly(dto.getUuid(), dto.getName(), dto.getUrl());
+    assertThat(newValue).hasToString("{\"webhookUuid\": \"" + dto.getUuid() + "\", \"name\": \"a-fancy-webhook\", \"url\": \"http://www.fancy-webhook.io\" }");
+  }
+
+  @Test
   public void updateProjectWebhookIsPersisted() {
     WebhookDto dto = webhookDbTester.insertGlobalWebhook();
     dto = dto