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

6 files changed, 51 insertions, 8 deletions

diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java
index 97230b61f9b..ad11767e0c5 100644
--- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java
+++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java
@@ -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;
diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java
index 62500919c5e..85966c3c136 100644
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java
@@ -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);
diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserRegistrarImplTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserRegistrarImplTest.java
index 46ddd229898..fdcb33b13c9 100644
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserRegistrarImplTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserRegistrarImplTest.java
@@ -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
diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java
index 6bb357642d4..a0f2702509f 100644
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java
@@ -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() {
diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java
index 25c55506b0d..1ad7b464ba0 100644
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java
@@ -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
diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java
index dae4966e208..84178855129 100644
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java
@@ -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