]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-15287 When updating a secret, audit logs should log the action
authorZipeng WU <zipeng.wu@sonarsource.com>
Wed, 18 Aug 2021 07:50:42 +0000 (09:50 +0200)
committersonartech <sonartech@sonarsource.com>
Thu, 19 Aug 2021 20:08:15 +0000 (20:08 +0000)
23 files changed:
server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDao.java
server/sonar-db-dao/src/main/java/org/sonar/db/audit/AuditPersister.java
server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/SecretNewValue.java [new file with mode: 0644]
server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDao.java
server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoTest.java
server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoWithPersisterTest.java
server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoWithPersisterTest.java
server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java
server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java
server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserRegistrarImplTest.java
server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java
server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java
server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateAzureAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateBitbucketAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateBitbucketCloudAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateGithubAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateGitlabAction.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/CreateActionTest.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateActionTest.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateIdentityProviderActionTest.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateLoginActionTest.java

index 973e7e82f446e48a4cae6213427148d6f8609088..e623b6407ebe4a0cf872bad436149c7f765a35e0 100644 (file)
@@ -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));
     }
   }
index 4a393c6ea769258b3e33f34f1b3cc542dbe04b80..ef4b852a6f51a598e36a98560c87386ef4812398 100644 (file)
@@ -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 (file)
index 0000000..85e47a3
--- /dev/null
@@ -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);
+  }
+}
index 43e962d7f971fc512480a5e484dd8ffd2a97df36..38c6be763294e69001ac9b2bce6d4ee0c275baa2 100644 (file)
@@ -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));
     }
   }
index b639e027b91c166e751767852975be0666ba41aa..b2b3a20797b89ef099e66c5c567646d167bfbc8f 100644 (file)
@@ -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)
index 9d734652e28770681a9cf136f9bfc0b13a4feea2..c88d3f50dfdfae0de64eab50c3a09a9133f6075d 100644 (file)
@@ -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
index 47b0f29498d1da17f4e5a953b79312c28b99a4e5..cb3429d319f1b5ee44dacf34a5dfdc7a163a7b0d 100644 (file)
@@ -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)
@@ -109,6 +115,28 @@ public class WebhookDaoWithPersisterTest {
     assertThat(newValue).hasToString("{\"webhookUuid\": \"" + dto.getUuid() + "\", \"name\": \"a-fancy-webhook\", \"url\": \"http://www.fancy-webhook.io\" }");
   }
 
+  @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();
index 97230b61f9b7bdcd3dea0b76bf5bf1efb36a49e5..ad11767e0c5b2f02bc26b4babcb478a2ceaa6adb 100644 (file)
@@ -35,6 +35,8 @@ import org.sonar.api.platform.NewUserHandler;
 import org.sonar.api.server.ServerSide;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
+import org.sonar.db.audit.AuditPersister;
+import org.sonar.db.audit.model.SecretNewValue;
 import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.db.user.UserGroupDto;
@@ -75,6 +77,8 @@ public class UserUpdater {
   private final UserIndexer userIndexer;
   private final DefaultGroupFinder defaultGroupFinder;
   private final Configuration config;
+  private final AuditPersister auditPersister;
+
   private final CredentialsLocalAuthentication localAuthentication;
 
   public UserUpdater(NewUserNotifier newUserNotifier, DbClient dbClient, UserIndexer userIndexer, DefaultGroupFinder defaultGroupFinder, Configuration config,
@@ -84,6 +88,18 @@ public class UserUpdater {
     this.userIndexer = userIndexer;
     this.defaultGroupFinder = defaultGroupFinder;
     this.config = config;
+    this.auditPersister = null;
+    this.localAuthentication = localAuthentication;
+  }
+
+  public UserUpdater(NewUserNotifier newUserNotifier, DbClient dbClient, UserIndexer userIndexer, DefaultGroupFinder defaultGroupFinder, Configuration config,
+    AuditPersister auditPersister, CredentialsLocalAuthentication localAuthentication) {
+    this.newUserNotifier = newUserNotifier;
+    this.dbClient = dbClient;
+    this.userIndexer = userIndexer;
+    this.defaultGroupFinder = defaultGroupFinder;
+    this.config = config;
+    this.auditPersister = auditPersister;
     this.localAuthentication = localAuthentication;
   }
 
@@ -195,7 +211,7 @@ public class UserUpdater {
     changed |= updateName(update, dto, messages);
     changed |= updateEmail(update, dto, messages);
     changed |= updateExternalIdentity(dbSession, update, dto);
-    changed |= updatePassword(update, dto, messages);
+    changed |= updatePassword(dbSession, update, dto, messages);
     changed |= updateScmAccounts(dbSession, update, dto, messages);
     checkRequest(messages.isEmpty(), messages);
     return changed;
@@ -244,11 +260,14 @@ public class UserUpdater {
     return false;
   }
 
-  private boolean updatePassword(UpdateUser updateUser, UserDto userDto, List<String> messages) {
+  private boolean updatePassword(DbSession dbSession, UpdateUser updateUser, UserDto userDto, List<String> messages) {
     String password = updateUser.password();
     if (updateUser.isPasswordChanged() && validatePasswords(password, messages) && checkPasswordChangeAllowed(userDto, messages)) {
       localAuthentication.storeHashPassword(userDto, password);
       userDto.setResetPassword(false);
+      if (auditPersister != null) {
+        auditPersister.updateUserPassword(dbSession, new SecretNewValue("userLogin", userDto.getLogin()));
+      }
       return true;
     }
     return false;
index 62500919c5ef199a7b7ac8a7b597aebf2e4aef5c..85966c3c1364c6fee58aa07ea4c92c0fe2bbe08f 100644 (file)
@@ -35,6 +35,7 @@ import org.sonar.api.config.internal.MapSettings;
 import org.sonar.api.impl.utils.AlwaysIncreasingSystem2;
 import org.sonar.api.utils.System2;
 import org.sonar.db.DbTester;
+import org.sonar.db.audit.AuditPersister;
 import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.server.authentication.event.AuthenticationEvent;
@@ -98,7 +99,7 @@ public class HttpHeadersAuthenticationTest {
 
   private final DefaultGroupFinder defaultGroupFinder = new DefaultGroupFinder(db.getDbClient());
   private final UserRegistrarImpl userIdentityAuthenticator = new UserRegistrarImpl(db.getDbClient(),
-    new UserUpdater(mock(NewUserNotifier.class), db.getDbClient(), userIndexer, defaultGroupFinder, settings.asConfig(), localAuthentication),
+    new UserUpdater(mock(NewUserNotifier.class), db.getDbClient(), userIndexer, defaultGroupFinder, settings.asConfig(), mock(AuditPersister.class), localAuthentication),
     defaultGroupFinder);
   private final HttpServletResponse response = mock(HttpServletResponse.class);
   private final JwtHttpHandler jwtHttpHandler = mock(JwtHttpHandler.class);
index 46ddd229898075774028b47e29b3db80eb1afd75..fdcb33b13c908c8efada92bac305a02624efd280 100644 (file)
@@ -32,6 +32,7 @@ import org.sonar.api.impl.utils.AlwaysIncreasingSystem2;
 import org.sonar.api.server.authentication.UserIdentity;
 import org.sonar.api.utils.log.LogTester;
 import org.sonar.db.DbTester;
+import org.sonar.db.audit.AuditPersister;
 import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.server.authentication.event.AuthenticationEvent;
@@ -46,7 +47,10 @@ import org.sonar.server.usergroups.DefaultGroupFinder;
 import static java.util.Arrays.stream;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
 import static org.sonar.db.user.UserTesting.newUserDto;
 import static org.sonar.process.ProcessProperties.Property.ONBOARDING_TUTORIAL_SHOW_TO_NEW_USERS;
 import static org.sonar.server.authentication.event.AuthenticationEvent.Method.BASIC;
@@ -82,7 +86,8 @@ public class UserRegistrarImplTest {
   private final UserIndexer userIndexer = new UserIndexer(db.getDbClient(), es.client());
   private final CredentialsLocalAuthentication localAuthentication = new CredentialsLocalAuthentication(db.getDbClient(), settings.asConfig());
   private final DefaultGroupFinder groupFinder = new DefaultGroupFinder(db.getDbClient());
-  private final UserUpdater userUpdater = new UserUpdater(mock(NewUserNotifier.class), db.getDbClient(), userIndexer, groupFinder, settings.asConfig(), localAuthentication);
+  private final AuditPersister auditPersister = mock(AuditPersister.class);
+  private final UserUpdater userUpdater = new UserUpdater(mock(NewUserNotifier.class), db.getDbClient(), userIndexer, groupFinder, settings.asConfig(), auditPersister, localAuthentication);
 
   private final UserRegistrarImpl underTest = new UserRegistrarImpl(db.getDbClient(), userUpdater, groupFinder);
   private GroupDto defaultGroup;
@@ -334,6 +339,7 @@ public class UserRegistrarImplTest {
         UserDto::isActive)
       .containsExactly("old login", USER_IDENTITY.getName(), USER_IDENTITY.getEmail(), USER_IDENTITY.getProviderId(), USER_IDENTITY.getProviderLogin(),
         IDENTITY_PROVIDER.getKey(), true);
+    verify(auditPersister, never()).updateUserPassword(any(), any());
   }
 
   @Test
index 6bb357642d42ab444a8d1ce3ce4296f5b29aa69e..a0f2702509ffbd4a7f57aaf1b6d6276a886c0e66 100644 (file)
@@ -35,6 +35,7 @@ import org.sonar.api.utils.System2;
 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.user.GroupDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.server.authentication.CredentialsLocalAuthentication;
@@ -77,7 +78,7 @@ public class UserUpdaterCreateTest {
   private final MapSettings settings = new MapSettings().setProperty("sonar.internal.pbkdf2.iterations", "1");
   private final CredentialsLocalAuthentication localAuthentication = new CredentialsLocalAuthentication(db.getDbClient(), settings.asConfig());
   private final UserUpdater underTest = new UserUpdater(newUserNotifier, dbClient, userIndexer,
-    new DefaultGroupFinder(dbClient), settings.asConfig(), localAuthentication);
+    new DefaultGroupFinder(dbClient), settings.asConfig(), null, localAuthentication);
 
   @Test
   public void create_user() {
index 25c55506b0d628313f577cb15143ccb7927405ca..1ad7b464ba071f1bcdd528db07ec09abe47fd6f1 100644 (file)
@@ -29,6 +29,7 @@ import org.sonar.api.utils.System2;
 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.user.GroupDto;
 import org.sonar.db.user.GroupTesting;
 import org.sonar.db.user.UserDto;
@@ -41,7 +42,11 @@ import org.sonar.server.usergroups.DefaultGroupFinder;
 import static java.lang.String.format;
 import static java.util.Collections.singletonList;
 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.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.sonar.process.ProcessProperties.Property.ONBOARDING_TUTORIAL_SHOW_TO_NEW_USERS;
 
 public class UserUpdaterReactivateTest {
@@ -61,9 +66,10 @@ public class UserUpdaterReactivateTest {
   private final UserIndexer userIndexer = new UserIndexer(dbClient, es.client());
   private final MapSettings settings = new MapSettings().setProperty("sonar.internal.pbkdf2.iterations", "1");
   private final CredentialsLocalAuthentication localAuthentication = new CredentialsLocalAuthentication(db.getDbClient(), settings.asConfig());
+  private final AuditPersister auditPersister = mock(AuditPersister.class);
   private final UserUpdater underTest = new UserUpdater(newUserNotifier, dbClient, userIndexer,
     new DefaultGroupFinder(dbClient),
-    settings.asConfig(), localAuthentication);
+    settings.asConfig(), auditPersister, localAuthentication);
 
   @Test
   public void reactivate_user() {
@@ -91,6 +97,7 @@ public class UserUpdaterReactivateTest {
     assertThat(reloaded.getCryptedPassword()).isNotNull().isNotEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg");
     assertThat(reloaded.getCreatedAt()).isEqualTo(user.getCreatedAt());
     assertThat(reloaded.getUpdatedAt()).isGreaterThan(user.getCreatedAt());
+    verify(auditPersister, times(1)).updateUserPassword(any(), any());
   }
 
   @Test
@@ -130,6 +137,7 @@ public class UserUpdaterReactivateTest {
     assertThat(dto.getCryptedPassword()).isNull();
     assertThat(dto.getCreatedAt()).isEqualTo(user.getCreatedAt());
     assertThat(dto.getUpdatedAt()).isGreaterThan(user.getCreatedAt());
+    verify(auditPersister, never()).updateUserPassword(any(), any());
   }
 
   @Test
index dae4966e208da6237702981bbb025376c5d5e83e..841788551292b855e68e7705213643dd0efcc164 100644 (file)
@@ -31,6 +31,7 @@ import org.sonar.api.utils.System2;
 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.component.ComponentDto;
 import org.sonar.db.property.PropertyDto;
 import org.sonar.db.property.PropertyQuery;
@@ -49,7 +50,11 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.tuple;
 import static org.assertj.core.data.MapEntry.entry;
+import static org.mockito.ArgumentMatchers.any;
 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.sonar.api.CoreProperties.DEFAULT_ISSUE_ASSIGNEE;
 import static org.sonar.db.user.UserTesting.newExternalUser;
 import static org.sonar.db.user.UserTesting.newLocalUser;
@@ -74,11 +79,12 @@ public class UserUpdaterUpdateTest {
   private final UserIndexer userIndexer = new UserIndexer(dbClient, es.client());
   private final MapSettings settings = new MapSettings().setProperty("sonar.internal.pbkdf2.iterations", "1");
   private final CredentialsLocalAuthentication localAuthentication = new CredentialsLocalAuthentication(db.getDbClient(), settings.asConfig());
+  private final AuditPersister auditPersister = mock(AuditPersister.class);
   private final UserUpdater underTest = new UserUpdater(newUserNotifier, dbClient, userIndexer,
-    new DefaultGroupFinder(dbClient), settings.asConfig(), localAuthentication);
+    new DefaultGroupFinder(dbClient), settings.asConfig(), auditPersister, localAuthentication);
 
   @Test
-  public void update_user() {
+  public void update_user_without_password() {
     UserDto user = db.users().insertUser(newLocalUser(DEFAULT_LOGIN, "Marius", "marius@email.com")
       .setScmAccounts(asList("ma", "marius33")));
     createDefaultGroup();
@@ -105,6 +111,7 @@ public class UserUpdaterUpdateTest {
         entry("login", DEFAULT_LOGIN),
         entry("name", "Marius2"),
         entry("email", "marius2@mail.com"));
+    verify(auditPersister, never()).updateUserPassword(any(), any());
   }
 
   @Test
@@ -161,6 +168,7 @@ public class UserUpdaterUpdateTest {
 
     UserDto dto = dbClient.userDao().selectByLogin(session, DEFAULT_LOGIN);
     assertThat(dto.getScmAccountsAsList()).containsOnly("ma2");
+    verify(auditPersister, times(1)).updateUserPassword(any(), any());
   }
 
   @Test
index 036122a243750a57cdd10eb840dc14e058bcbf7e..111f6306dafc9feb6bccda3b3e137b66bcb1cc2e 100644 (file)
@@ -97,7 +97,8 @@ public class UpdateAzureAction implements AlmSettingsWsAction {
       dbClient.almSettingDao().update(dbSession, almSettingDto
         .setKey(isNotBlank(newKey) ? newKey : key)
         .setPersonalAccessToken(isNotBlank(pat) ? pat : almSettingDto.getPersonalAccessToken())
-        .setUrl(url));
+        .setUrl(url),
+        pat != null);
       dbSession.commit();
     }
   }
index 5d5fb161702d34339586f53326b0c31c3272066e..811621fb64c939693dd0e871189cf8b0f2a5b086 100644 (file)
@@ -96,7 +96,8 @@ public class UpdateBitbucketAction implements AlmSettingsWsAction {
       dbClient.almSettingDao().update(dbSession, almSettingDto
         .setKey(isNotBlank(newKey) ? newKey : key)
         .setUrl(url)
-        .setPersonalAccessToken(isNotBlank(pat) ? pat : almSettingDto.getPersonalAccessToken()));
+        .setPersonalAccessToken(isNotBlank(pat) ? pat : almSettingDto.getPersonalAccessToken()),
+        pat != null);
       dbSession.commit();
     }
   }
index ab7cbce84d56c504b58b3054ef20357cee38d2c1..1f00edcec0fe3a2d2b4a58a559cba7b2c9471521 100644 (file)
@@ -100,7 +100,8 @@ public class UpdateBitbucketCloudAction implements AlmSettingsWsAction {
         .setKey(isNotBlank(newKey) ? newKey : key)
         .setClientId(clientId)
         .setAppId(workspace)
-        .setClientSecret(isNotBlank(clientSecret) ? clientSecret : almSettingDto.getClientSecret()));
+        .setClientSecret(isNotBlank(clientSecret) ? clientSecret : almSettingDto.getClientSecret()),
+        clientSecret != null);
       dbSession.commit();
     }
   }
index 0bf824ec53d870924b8939c2fd1c459deb5d4128..f9667cb1c9dfc7c54473c31fbddb181d57684273 100644 (file)
@@ -122,7 +122,8 @@ public class UpdateGithubAction implements AlmSettingsWsAction {
         .setAppId(appId)
         .setPrivateKey(isNotBlank(privateKey) ? privateKey : almSettingDto.getPrivateKey())
         .setClientId(clientId)
-        .setClientSecret(isNotBlank(clientSecret) ? clientSecret : almSettingDto.getClientSecret()));
+        .setClientSecret(isNotBlank(clientSecret) ? clientSecret : almSettingDto.getClientSecret()),
+        clientSecret != null || privateKey != null);
       dbSession.commit();
     }
   }
index 09ced8e74be7630d48d1e38a537064052c52ca38..2ef16921f0087f7acedc6597d7f4328ca9aa694f 100644 (file)
@@ -98,7 +98,8 @@ public class UpdateGitlabAction implements AlmSettingsWsAction {
       dbClient.almSettingDao().update(dbSession, almSettingDto
         .setKey(isNotBlank(newKey) ? newKey : key)
         .setUrl(url)
-        .setPersonalAccessToken(isNotBlank(pat) ? pat : almSettingDto.getPersonalAccessToken()));
+        .setPersonalAccessToken(isNotBlank(pat) ? pat : almSettingDto.getPersonalAccessToken()),
+        pat != null);
       dbSession.commit();
     }
   }
index 9648c2db8ed33cb3ac3f2e7bd83fbb33fd16663f..0c11cdee05b6fd1be7a1c40c32e3c1f4c3b3dd30 100644 (file)
@@ -59,7 +59,7 @@ public class ChangePasswordActionTest {
 
   private final UserUpdater userUpdater = new UserUpdater(mock(NewUserNotifier.class), db.getDbClient(),
     new UserIndexer(db.getDbClient(), es.client()), new DefaultGroupFinder(db.getDbClient()),
-    new MapSettings().asConfig(), localAuthentication);
+    new MapSettings().asConfig(), null, localAuthentication);
 
   private final WsActionTester tester = new WsActionTester(new ChangePasswordAction(db.getDbClient(), userUpdater, userSessionRule, localAuthentication));
 
index d17a60f9b54e3dec322f59d8c9277f68e2d056c8..e4fea46985054151c4919bbf171bdf550ba5b2ff 100644 (file)
@@ -79,7 +79,7 @@ public class CreateActionTest {
   private GroupDto defaultGroup;
   private final CredentialsLocalAuthentication localAuthentication = new CredentialsLocalAuthentication(db.getDbClient(), settings.asConfig());
   private final WsActionTester tester = new WsActionTester(new CreateAction(db.getDbClient(), new UserUpdater(mock(NewUserNotifier.class),
-    db.getDbClient(), userIndexer, new DefaultGroupFinder(db.getDbClient()), settings.asConfig(), localAuthentication), userSessionRule));
+    db.getDbClient(), userIndexer, new DefaultGroupFinder(db.getDbClient()), settings.asConfig(), null, localAuthentication), userSessionRule));
 
   @Before
   public void setUp() {
index c9eced59f3836b4aad399f8eb10aa8c45fa6ffb9..3beb0e68546e22a54d0ef8dd0b293e80090ec337 100644 (file)
@@ -67,7 +67,7 @@ public class UpdateActionTest {
   private final UserIndexer userIndexer = new UserIndexer(dbClient, es.client());
   private final CredentialsLocalAuthentication localAuthentication = new CredentialsLocalAuthentication(db.getDbClient(), settings.asConfig());
   private final WsActionTester ws = new WsActionTester(new UpdateAction(
-    new UserUpdater(mock(NewUserNotifier.class), dbClient, userIndexer, new DefaultGroupFinder(db.getDbClient()), settings.asConfig(), localAuthentication),
+    new UserUpdater(mock(NewUserNotifier.class), dbClient, userIndexer, new DefaultGroupFinder(db.getDbClient()), settings.asConfig(), null, localAuthentication),
     userSession, new UserJsonWriter(userSession), dbClient));
 
   @Before
index a7425109bc462c187a61d4844f05d13bc06e976b..3693f1006d01a4c7bfd2b929bffdce2ed39688f1 100644 (file)
@@ -69,7 +69,7 @@ public class UpdateIdentityProviderActionTest {
   private final CredentialsLocalAuthentication localAuthentication = new CredentialsLocalAuthentication(dbClient, settings.asConfig());
 
   private final WsActionTester underTest = new WsActionTester(new UpdateIdentityProviderAction(dbClient, identityProviderRepository,
-    new UserUpdater(mock(NewUserNotifier.class), dbClient, userIndexer, new DefaultGroupFinder(db.getDbClient()), settings.asConfig(), localAuthentication),
+    new UserUpdater(mock(NewUserNotifier.class), dbClient, userIndexer, new DefaultGroupFinder(db.getDbClient()), settings.asConfig(), null, localAuthentication),
     userSession));
 
   @Test
index cc7a5dfe2444ba77621bbffd855d149435aca527..7055fe9e3e8caa34aaf4221d886f3986473f1f96 100644 (file)
@@ -56,7 +56,7 @@ public class UpdateLoginActionTest {
   public ExpectedException expectedException = ExpectedException.none();
 
   private final WsActionTester ws = new WsActionTester(new UpdateLoginAction(db.getDbClient(), userSession,
-    new UserUpdater(mock(NewUserNotifier.class), db.getDbClient(), new UserIndexer(db.getDbClient(), es.client()), null, null, null)));
+    new UserUpdater(mock(NewUserNotifier.class), db.getDbClient(), new UserIndexer(db.getDbClient(), es.client()), null, null, null, null)));
 
   @Test
   public void update_login_from_sonarqube_account_when_user_is_local() {