From b4d312cea6b76d2103fce569296a8215d3a6edc3 Mon Sep 17 00:00:00 2001 From: Zipeng WU Date: Wed, 18 Aug 2021 09:50:42 +0200 Subject: [PATCH] SONAR-15287 When updating a secret, audit logs should log the action --- .../sonar/db/alm/setting/AlmSettingDao.java | 6 +++- .../org/sonar/db/audit/AuditPersister.java | 6 ++++ .../sonar/db/audit/model/SecretNewValue.java | 35 +++++++++++++++++++ .../java/org/sonar/db/webhook/WebhookDao.java | 5 ++- .../db/alm/setting/AlmSettingDaoTest.java | 2 +- .../AlmSettingDaoWithPersisterTest.java | 15 +++++--- .../webhook/WebhookDaoWithPersisterTest.java | 30 +++++++++++++++- .../org/sonar/server/user/UserUpdater.java | 23 ++++++++++-- .../HttpHeadersAuthenticationTest.java | 3 +- .../authentication/UserRegistrarImplTest.java | 8 ++++- .../server/user/UserUpdaterCreateTest.java | 3 +- .../user/UserUpdaterReactivateTest.java | 10 +++++- .../server/user/UserUpdaterUpdateTest.java | 12 +++++-- .../almsettings/ws/UpdateAzureAction.java | 3 +- .../almsettings/ws/UpdateBitbucketAction.java | 3 +- .../ws/UpdateBitbucketCloudAction.java | 3 +- .../almsettings/ws/UpdateGithubAction.java | 3 +- .../almsettings/ws/UpdateGitlabAction.java | 3 +- .../user/ws/ChangePasswordActionTest.java | 2 +- .../server/user/ws/CreateActionTest.java | 2 +- .../server/user/ws/UpdateActionTest.java | 2 +- .../ws/UpdateIdentityProviderActionTest.java | 2 +- .../server/user/ws/UpdateLoginActionTest.java | 2 +- 23 files changed, 157 insertions(+), 26 deletions(-) create mode 100644 server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/SecretNewValue.java 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 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 newValueCaptor = ArgumentCaptor.forClass(WebhookNewValue.class); + private final ArgumentCaptor 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(); 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 messages) { + private boolean updatePassword(DbSession dbSession, UpdateUser updateUser, UserDto userDto, List 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 diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateAzureAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateAzureAction.java index 036122a2437..111f6306daf 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateAzureAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateAzureAction.java @@ -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(); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateBitbucketAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateBitbucketAction.java index 5d5fb161702..811621fb64c 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateBitbucketAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateBitbucketAction.java @@ -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(); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateBitbucketCloudAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateBitbucketCloudAction.java index ab7cbce84d5..1f00edcec0f 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateBitbucketCloudAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateBitbucketCloudAction.java @@ -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(); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateGithubAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateGithubAction.java index 0bf824ec53d..f9667cb1c9d 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateGithubAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateGithubAction.java @@ -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(); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateGitlabAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateGitlabAction.java index 09ced8e74be..2ef16921f00 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateGitlabAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/UpdateGitlabAction.java @@ -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(); } } diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java index 9648c2db8ed..0c11cdee05b 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java @@ -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)); diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/CreateActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/CreateActionTest.java index d17a60f9b54..e4fea469850 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/CreateActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/CreateActionTest.java @@ -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() { diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateActionTest.java index c9eced59f38..3beb0e68546 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateActionTest.java @@ -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 diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateIdentityProviderActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateIdentityProviderActionTest.java index a7425109bc4..3693f1006d0 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateIdentityProviderActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateIdentityProviderActionTest.java @@ -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 diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateLoginActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateLoginActionTest.java index cc7a5dfe244..7055fe9e3e8 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateLoginActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UpdateLoginActionTest.java @@ -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() { -- 2.39.5