From 917574dcd1a9c4ea379cb22c65031b9b2f11fa97 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 17 Aug 2016 16:57:08 +0200 Subject: [PATCH] SONAR-7994 Clean up migration that feed local users --- .../src/test/java/it/Category5Suite.java | 4 +- ...Test.java => RealmAuthenticationTest.java} | 4 +- .../ComputeEngineContainerImplTest.java | 2 +- .../platformlevel/PlatformLevelStartup.java | 7 +- .../startup/FeedUsersLocalStartupTask.java | 141 ---------- .../FeedUsersLocalStartupTaskTest.java | 251 ------------------ .../core/config/CorePropertyDefinitions.java | 8 - 7 files changed, 7 insertions(+), 410 deletions(-) rename it/it-tests/src/test/java/it/user/{RailsExternalAuthenticationTest.java => RealmAuthenticationTest.java} (98%) delete mode 100644 server/sonar-server/src/main/java/org/sonar/server/startup/FeedUsersLocalStartupTask.java delete mode 100644 server/sonar-server/src/test/java/org/sonar/server/startup/FeedUsersLocalStartupTaskTest.java diff --git a/it/it-tests/src/test/java/it/Category5Suite.java b/it/it-tests/src/test/java/it/Category5Suite.java index 1e3af4ff8c2..7f5f85b2a7a 100644 --- a/it/it-tests/src/test/java/it/Category5Suite.java +++ b/it/it-tests/src/test/java/it/Category5Suite.java @@ -24,7 +24,7 @@ import it.serverSystem.RestartTest; import it.serverSystem.ServerSystemRestartingOrchestrator; import it.settings.SettingsTestRestartingOrchestrator; import it.updateCenter.UpdateCenterTest; -import it.user.RailsExternalAuthenticationTest; +import it.user.RealmAuthenticationTest; import org.junit.runner.RunWith; import org.junit.runners.Suite; @@ -41,7 +41,7 @@ import org.junit.runners.Suite; SettingsTestRestartingOrchestrator.class, // update center UpdateCenterTest.class, - RailsExternalAuthenticationTest.class + RealmAuthenticationTest.class }) public class Category5Suite { diff --git a/it/it-tests/src/test/java/it/user/RailsExternalAuthenticationTest.java b/it/it-tests/src/test/java/it/user/RealmAuthenticationTest.java similarity index 98% rename from it/it-tests/src/test/java/it/user/RailsExternalAuthenticationTest.java rename to it/it-tests/src/test/java/it/user/RealmAuthenticationTest.java index 0236652ee99..c61398ed44f 100644 --- a/it/it-tests/src/test/java/it/user/RailsExternalAuthenticationTest.java +++ b/it/it-tests/src/test/java/it/user/RealmAuthenticationTest.java @@ -56,11 +56,11 @@ import static util.ItUtils.pluginArtifact; import static util.ItUtils.setServerProperty; /** - * Test deprecated authentication done by Rails. It's kept has every features has not bee migrated to java yet. + * Test REALM authentication. * * It starts its own server as it's using a different authentication system */ -public class RailsExternalAuthenticationTest { +public class RealmAuthenticationTest { @Rule public ExpectedException thrown = ExpectedException.none(); diff --git a/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java b/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java index 9fc85e9f8a6..6a4709b1260 100644 --- a/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java +++ b/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java @@ -107,7 +107,7 @@ public class ComputeEngineContainerImplTest { + 26 // level 1 + 47 // content of DaoModule + 2 // content of EsSearchModule - + 55 // content of CorePropertyDefinitions + + 54 // content of CorePropertyDefinitions + 1 // content of CePropertyDefinitions ); assertThat(picoContainer.getParent().getParent().getParent().getParent()).isNull(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java index 67fa802f2ad..0d49dee0587 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java @@ -32,7 +32,6 @@ import org.sonar.server.rule.RegisterRules; import org.sonar.server.startup.ClearRulesOverloadedDebt; import org.sonar.server.startup.DeleteOldAnalysisReportsFromFs; import org.sonar.server.startup.DisplayLogOnDeprecatedProjects; -import org.sonar.server.startup.FeedUsersLocalStartupTask; import org.sonar.server.startup.GeneratePluginIndex; import org.sonar.server.startup.RegisterDashboards; import org.sonar.server.startup.RegisterMetrics; @@ -53,7 +52,7 @@ public class PlatformLevelStartup extends PlatformLevel { add(GeneratePluginIndex.class, RegisterServletFilters.class, ServerLifecycleNotifier.class); - + addIfStartupLeader( CheckDatabaseCollationDuringMigration.class, IndexerStartupTask.class, @@ -69,9 +68,7 @@ public class PlatformLevelStartup extends PlatformLevel { RenameIssueWidgets.class, DisplayLogOnDeprecatedProjects.class, ClearRulesOverloadedDebt.class, - DeleteOldAnalysisReportsFromFs.class, - FeedUsersLocalStartupTask.class); - + DeleteOldAnalysisReportsFromFs.class); } @Override diff --git a/server/sonar-server/src/main/java/org/sonar/server/startup/FeedUsersLocalStartupTask.java b/server/sonar-server/src/main/java/org/sonar/server/startup/FeedUsersLocalStartupTask.java deleted file mode 100644 index a5316f9b383..00000000000 --- a/server/sonar-server/src/main/java/org/sonar/server/startup/FeedUsersLocalStartupTask.java +++ /dev/null @@ -1,141 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2016 SonarSource SA - * mailto:contact 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.server.startup; - -import java.util.Arrays; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; -import org.picocontainer.Startable; -import org.sonar.api.config.Settings; -import org.sonar.api.security.SecurityRealm; -import org.sonar.api.user.UserQuery; -import org.sonar.api.utils.MessageException; -import org.sonar.api.utils.System2; -import org.sonar.api.utils.log.Logger; -import org.sonar.api.utils.log.Loggers; -import org.sonar.db.DbClient; -import org.sonar.db.DbSession; -import org.sonar.db.loadedtemplate.LoadedTemplateDto; -import org.sonar.db.user.UserDto; -import org.sonar.server.user.UserUpdater; - -import static java.util.Arrays.asList; -import static org.sonar.api.CoreProperties.CORE_AUTHENTICATOR_REALM; -import static org.sonar.db.loadedtemplate.LoadedTemplateDto.ONE_SHOT_TASK_TYPE; - -/** - * Feed users local property. - * If a realm is defined, then users are set as local only if their login are found in the property "sonar.security.localUsers", - * otherwise user are all set as local. - *

- * See SONAR-7254. - *

- * Should be removed after LTS 5.X - * - * @since 5.5 - */ -public class FeedUsersLocalStartupTask implements Startable { - - private static final Logger LOG = Loggers.get(FeedUsersLocalStartupTask.class); - - private static final String TEMPLATE_KEY = "UpdateUsersLocal"; - private static final String LOCAL_USERS_PROPERTY = "sonar.security.localUsers"; - - private final System2 system2; - - private final DbClient dbClient; - private final Settings settings; - private final SecurityRealm[] realms; - - /** - * Used when no realm plugin - */ - public FeedUsersLocalStartupTask(System2 system2, DbClient dbClient, Settings settings) { - this.system2 = system2; - this.dbClient = dbClient; - this.settings = settings; - this.realms = new SecurityRealm[]{}; - } - - public FeedUsersLocalStartupTask(System2 system2, DbClient dbClient, Settings settings, SecurityRealm[] realms) { - this.system2 = system2; - this.dbClient = dbClient; - this.settings = settings; - this.realms = realms; - } - - @Override - public void start() { - DbSession dbSession = dbClient.openSession(false); - try { - if (hasAlreadyBeenExecuted(dbSession)) { - return; - } - updateUsersLocal(dbSession); - markAsExecuted(dbSession); - dbSession.commit(); - - if (settings.hasKey(LOCAL_USERS_PROPERTY)) { - LOG.info("NOTE : The property '{}' is now no more needed, you can safely remove it.", LOCAL_USERS_PROPERTY); - } - } finally { - dbClient.closeSession(dbSession); - } - } - - private void updateUsersLocal(DbSession dbSession) { - boolean realmConfExists = settings.getString(CORE_AUTHENTICATOR_REALM) != null; - checkConfigurationIfRealmExists(realmConfExists); - long now = system2.now(); - Set localUsers = new HashSet<>(asList(settings.getStringArray(LOCAL_USERS_PROPERTY))); - for (UserDto user : dbClient.userDao().selectUsers(dbSession, UserQuery.ALL_ACTIVES)) { - if (user.getExternalIdentityProvider().equals(UserUpdater.SQ_AUTHORITY)) { - user.setLocal(!realmConfExists || localUsers.contains(user.getLogin())); - } else { - user.setLocal(false); - } - user.setUpdatedAt(now); - dbClient.userDao().update(dbSession, user); - } - } - - private void checkConfigurationIfRealmExists(boolean realmConfExists) { - List realmNames = Arrays.stream(realms).map(SecurityRealm::getName).collect(Collectors.toList()); - if (!realmNames.isEmpty() && !realmConfExists) { - throw MessageException.of(String.format("External authentication plugin %s has been found, but no related configuration has been set. " + - "Either update your configuration or remove the plugin", realmNames)); - } - } - - private boolean hasAlreadyBeenExecuted(DbSession dbSession) { - return dbClient.loadedTemplateDao().countByTypeAndKey(ONE_SHOT_TASK_TYPE, TEMPLATE_KEY, dbSession) > 0; - } - - private void markAsExecuted(DbSession dbSession) { - dbClient.loadedTemplateDao().insert(new LoadedTemplateDto(TEMPLATE_KEY, ONE_SHOT_TASK_TYPE), dbSession); - } - - @Override - public void stop() { - // Nothing to do - } -} diff --git a/server/sonar-server/src/test/java/org/sonar/server/startup/FeedUsersLocalStartupTaskTest.java b/server/sonar-server/src/test/java/org/sonar/server/startup/FeedUsersLocalStartupTaskTest.java deleted file mode 100644 index 3de4c5f72e5..00000000000 --- a/server/sonar-server/src/test/java/org/sonar/server/startup/FeedUsersLocalStartupTaskTest.java +++ /dev/null @@ -1,251 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2016 SonarSource SA - * mailto:contact 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.server.startup; - -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.sonar.api.config.Settings; -import org.sonar.api.security.SecurityRealm; -import org.sonar.api.utils.MessageException; -import org.sonar.api.utils.System2; -import org.sonar.api.utils.log.LogTester; -import org.sonar.api.utils.log.LoggerLevel; -import org.sonar.db.DbClient; -import org.sonar.db.DbSession; -import org.sonar.db.DbTester; -import org.sonar.db.user.UserDao; -import org.sonar.db.user.UserDto; -import org.sonar.db.user.UserTesting; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import static org.sonar.db.loadedtemplate.LoadedTemplateDto.ONE_SHOT_TASK_TYPE; - -public class FeedUsersLocalStartupTaskTest { - - static final long NOW = 20000000L; - static final long PAST = 10000000L; - - static final String USER1_LOGIN = "USER1"; - static final String USER2_LOGIN = "USER2"; - - static final String REALM_NAME = "LDAP"; - - @Rule - public ExpectedException expectedException = ExpectedException.none(); - - System2 system2 = mock(System2.class); - - @Rule - public LogTester logTester = new LogTester(); - - @Rule - public DbTester dbTester = DbTester.create(system2); - - DbClient dbClient = dbTester.getDbClient(); - - DbSession dbSession = dbTester.getSession(); - - UserDao userDao = dbClient.userDao(); - - Settings settings = new Settings(); - - FeedUsersLocalStartupTask underTest; - - @Before - public void setUp() throws Exception { - when(system2.now()).thenReturn(NOW); - } - - @Test - public void set_user_local_when_id_provider_is_sonarqube_and_realm_exists_and_local_users_property_contains_user_login() throws Exception { - initTaskWithRealm(); - settings.setProperty("sonar.security.realm", REALM_NAME); - settings.setProperty("sonar.security.localUsers", USER1_LOGIN); - UserDto user = addUser(USER1_LOGIN, false, "sonarqube"); - - underTest.start(); - - verifyUserIsUpdated(user.getId(), true); - verifyLogAboutRemovalOfLocalUsersProperty(); - } - - @Test - public void set_user_as_not_local_when_id_provider_is_sonarqube_and_ream_exists_and_no_local_users_property() throws Exception { - initTaskWithRealm(); - settings.setProperty("sonar.security.realm", REALM_NAME); - UserDto user = addUser(USER1_LOGIN, false, "sonarqube"); - - underTest.start(); - - verifyUserIsUpdated(user.getId(), false); - verifyEmptyLog(); - } - - @Test - public void set_user_as_not_local_when_id_provider_is_sonarqube_and_ream_exists_and_local_users_property_does_not_contain_user_login() throws Exception { - initTaskWithRealm(); - settings.setProperty("sonar.security.realm", REALM_NAME); - settings.setProperty("sonar.security.localUsers", USER2_LOGIN); - UserDto user = addUser(USER1_LOGIN, true, "sonarqube"); - - underTest.start(); - - verifyUserIsUpdated(user.getId(), false); - verifyLogAboutRemovalOfLocalUsersProperty(); - } - - @Test - public void set_user_as_local_when_id_provider_is_sonarqube_and_no_realm() throws Exception { - initTaskWithNoRealm(); - settings.setProperty("sonar.security.realm", (String) null); - UserDto user = addUser(USER1_LOGIN, false, "sonarqube"); - - underTest.start(); - - verifyUserIsUpdated(user.getId(), true); - verifyEmptyLog(); - } - - @Test - public void set_user_as_not_local_when_external_identity_is_not_sonarqube() throws Exception { - initTaskWithNoRealm(); - UserDto user = addUser(USER1_LOGIN, false, "github"); - - underTest.start(); - - verifyUserIsUpdated(user.getId(), false); - verifyEmptyLog(); - } - - @Test - public void does_not_update_removed_user() throws Exception { - initTaskWithRealm(); - settings.setProperty("sonar.security.realm", REALM_NAME); - - UserDto user = UserTesting.newUserDto() - .setLogin(USER1_LOGIN) - .setActive(false) - .setLocal(false) - .setCreatedAt(PAST) - .setUpdatedAt(PAST); - userDao.insert(dbSession, user); - dbSession.commit(); - - underTest.start(); - - UserDto userReloaded = userDao.selectUserById(dbSession, user.getId()); - assertThat(userReloaded.isLocal()).isFalse(); - assertThat(userReloaded.getUpdatedAt()).isEqualTo(PAST); - verifyTaskIsRegistered(); - } - - @Test - public void does_nothing_when_task_has_already_been_executed() throws Exception { - initTaskWithRealm(); - settings.setProperty("sonar.security.realm", REALM_NAME); - settings.setProperty("sonar.security.localUsers", USER1_LOGIN); - UserDto user = addUser(USER1_LOGIN, false, "github"); - - underTest.start(); - verifyLogAboutRemovalOfLocalUsersProperty(); - - logTester.clear(); - UserDto userReloaded = userDao.selectUserById(dbSession, user.getId()); - assertThat(userReloaded.getUpdatedAt()).isEqualTo(NOW); - verifyTaskIsRegistered(); - - when(system2.now()).thenReturn(NOW + 1000L); - - underTest.start(); - userReloaded = userDao.selectUserById(dbSession, user.getId()); - assertThat(userReloaded.getUpdatedAt()).isEqualTo(NOW); - verifyEmptyLog(); - } - - @Test - public void fail_when_realm_found_but_no_configuration() throws Exception { - initTask(createRealm("REALM1"), createRealm("REALM2")); - - expectedException.expect(MessageException.class); - expectedException.expectMessage("External authentication plugin [REALM1, REALM2] has been found, but no related configuration has been set. " + - "Either update your configuration or remove the plugin"); - underTest.start(); - } - - private UserDto addUser(String login, boolean local, String externalIdentityProvider) { - UserDto user = UserTesting.newUserDto() - .setLogin(login) - .setActive(true) - .setLocal(local) - .setExternalIdentityProvider(externalIdentityProvider) - .setExternalIdentity(login) - .setCreatedAt(PAST) - .setUpdatedAt(PAST); - userDao.insert(dbSession, user); - dbSession.commit(); - return user; - } - - private void verifyUserIsUpdated(long userId, boolean expectedLocal) { - UserDto userReloaded = userDao.selectUserById(dbSession, userId); - assertThat(userReloaded.isLocal()).isEqualTo(expectedLocal); - assertThat(userReloaded.getUpdatedAt()).isEqualTo(NOW); - verifyTaskIsRegistered(); - } - - private void verifyTaskIsRegistered() { - assertThat(dbClient.loadedTemplateDao().countByTypeAndKey(ONE_SHOT_TASK_TYPE, "UpdateUsersLocal")).isEqualTo(1); - } - - private void verifyLogAboutRemovalOfLocalUsersProperty() { - assertThat(logTester.logs(LoggerLevel.INFO)).containsOnly( - "NOTE : The property 'sonar.security.localUsers' is now no more needed, you can safely remove it."); - } - - private void verifyEmptyLog() { - assertThat(logTester.logs(LoggerLevel.INFO)).isEmpty(); - } - - private void initTask(SecurityRealm... realms) { - if (realms.length == 0) { - underTest = new FeedUsersLocalStartupTask(system2, dbTester.getDbClient(), settings); - } else { - underTest = new FeedUsersLocalStartupTask(system2, dbTester.getDbClient(), settings, realms); - } - } - - private void initTaskWithRealm() { - initTask(createRealm(REALM_NAME)); - } - - private void initTaskWithNoRealm() { - initTask(); - } - - private static SecurityRealm createRealm(String name) { - SecurityRealm realm = mock(SecurityRealm.class); - when(realm.getName()).thenReturn(name); - return realm; - } -} diff --git a/sonar-core/src/main/java/org/sonar/core/config/CorePropertyDefinitions.java b/sonar-core/src/main/java/org/sonar/core/config/CorePropertyDefinitions.java index d0728366983..5c173cae78d 100644 --- a/sonar-core/src/main/java/org/sonar/core/config/CorePropertyDefinitions.java +++ b/sonar-core/src/main/java/org/sonar/core/config/CorePropertyDefinitions.java @@ -146,14 +146,6 @@ public class CorePropertyDefinitions { .type(PropertyType.BOOLEAN) .defaultValue(String.valueOf(false)) .build(), - PropertyDefinition.builder(CoreProperties.CORE_AUTHENTICATOR_LOCAL_USERS) - .name("Local/technical users") - .description("Comma separated list of user logins that will always be authenticated using SonarQube database. " - + "When using the LDAP plugin, for these accounts, the user attributes (name, email, ...) are not re-synchronized") - .type(PropertyType.STRING) - .multiValues(true) - .defaultValue("admin") - .build(), PropertyDefinition.builder(CoreProperties.SCM_DISABLED_KEY) .name("Disable the SCM Sensor") .description("Disable the retrieval of blame information from Source Control Manager") -- 2.39.5