From 6e18f97ab530ffc932b1b2a2888e31e112b4be96 Mon Sep 17 00:00:00 2001 From: Jacek Date: Wed, 18 Nov 2020 10:08:33 +0100 Subject: [PATCH] SONAR-14159 enforce user authentication by default --- .../java/org/sonar/db/SQDatabase.java | 1 + .../version/v00/PopulateInitialSchema.java | 17 +++ .../db/migration/version/v86/DbVersion86.java | 1 + .../v86/SetForceAuthenticationSettings.java | 65 ++++++++++ .../v00/PopulateInitialSchemaTest.java | 88 ++++++++----- .../SetForceAuthenticationSettingsTest.java | 120 ++++++++++++++++++ .../schema.sql | 12 ++ .../UserSessionInitializer.java | 3 +- .../UserSessionInitializerTest.java | 26 +++- .../monitoring/StandaloneSystemSection.java | 3 +- .../cluster/GlobalSystemSection.java | 3 +- .../StandaloneSystemSectionTest.java | 16 ++- .../cluster/GlobalSystemSectionTest.java | 14 ++ .../authentication/ws/ValidateAction.java | 3 +- .../authentication/ws/ValidateActionTest.java | 11 ++ .../sonar/core/config/SecurityProperties.java | 3 +- .../core/config/SecurityPropertiesTest.java | 42 ++++++ .../java/org/sonar/api/CoreProperties.java | 2 +- 18 files changed, 385 insertions(+), 45 deletions(-) create mode 100644 server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v86/SetForceAuthenticationSettings.java create mode 100644 server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v86/SetForceAuthenticationSettingsTest.java create mode 100644 server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v86/SetForceAuthenticationSettingsTest/schema.sql create mode 100644 sonar-core/src/test/java/org/sonar/core/config/SecurityPropertiesTest.java diff --git a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/SQDatabase.java b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/SQDatabase.java index 60ecaba510b..9394e63f739 100644 --- a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/SQDatabase.java +++ b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/SQDatabase.java @@ -158,6 +158,7 @@ public class SQDatabase extends DefaultDatabase { parentContainer.add(SonarRuntimeImpl.forSonarQube(Version.create(8, 0), SonarQubeSide.SERVER, SonarEdition.COMMUNITY)); parentContainer.add(UuidFactoryFast.getInstance()); parentContainer.add(System2.INSTANCE); + parentContainer.add(MapSettings.class); parentContainer.startComponents(); diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchema.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchema.java index da98dee308e..0e971237b62 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchema.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchema.java @@ -64,6 +64,7 @@ public class PopulateInitialSchema extends DataChange { insertOrganization(context, organizationUuid, groups, defaultQGUuid); insertOrgQualityGate(context, organizationUuid, defaultQGUuid); insertInternalProperty(context, organizationUuid); + insertPropertyToEnableForceAuthentication(context); insertGroupRoles(context, organizationUuid, groups); insertGroupUsers(context, adminUserId, groups); insertOrganizationMember(context, adminUserId, organizationUuid); @@ -153,6 +154,22 @@ public class PopulateInitialSchema extends DataChange { .commit(); } + private void insertPropertyToEnableForceAuthentication(Context context) throws SQLException { + String tableName = "properties"; + truncateTable(context, tableName); + + long now = system2.now(); + Upsert upsert = context.prepareUpsert(createInsertStatement(tableName, "prop_key", "is_empty", "text_value", "created_at")); + upsert + .setString(1, "sonar.forceAuthentication") + .setBoolean(2, false) + .setString(3, "true") + .setLong(4, now); + upsert + .execute() + .commit(); + } + private Groups insertGroups(Context context, String organizationUuid) throws SQLException { truncateTable(context, "groups"); diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v86/DbVersion86.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v86/DbVersion86.java index 9e2283de50d..9429fc93a39 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v86/DbVersion86.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v86/DbVersion86.java @@ -42,6 +42,7 @@ public class DbVersion86 implements DbVersion { .add(4112, "Make 'name' column in 'groups' table not nullable", MakeNameColumnInGroupsTableNotNullable.class) .add(4113, "Make 'name' column in 'groups' table unique", AddUniqueIndexOnNameColumnOfGroupsTable.class) .add(4114, "Move default permission templates to internal properties", MoveDefaultTemplatesToInternalProperties.class) + .add(4115, "Set 'sonar.forceAuthentication' to false for upgraded instances", SetForceAuthenticationSettings.class) ; } } diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v86/SetForceAuthenticationSettings.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v86/SetForceAuthenticationSettings.java new file mode 100644 index 00000000000..dee8b7b4a63 --- /dev/null +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v86/SetForceAuthenticationSettings.java @@ -0,0 +1,65 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 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.server.platform.db.migration.version.v86; + +import java.sql.SQLException; +import java.util.Optional; +import org.sonar.api.config.internal.Settings; +import org.sonar.api.utils.System2; +import org.sonar.core.util.UuidFactory; +import org.sonar.db.Database; +import org.sonar.server.platform.db.migration.step.DataChange; +import org.sonar.server.platform.db.migration.step.Select; + +public class SetForceAuthenticationSettings extends DataChange { + private static final String PROPERTY_NAME_FORCE_AUTHENTICATION = "sonar.forceAuthentication"; + private System2 system2; + private UuidFactory uuidFactory; + private Settings settings; + + public SetForceAuthenticationSettings(Database db, System2 system2, UuidFactory uuidFactory, Settings settings) { + super(db); + this.system2 = system2; + this.uuidFactory = uuidFactory; + this.settings = settings; + } + + @Override + protected void execute(Context context) throws SQLException { + Select select = context.prepareSelect("select p.text_value from properties p where p.prop_key = ?") + .setString(1, PROPERTY_NAME_FORCE_AUTHENTICATION); + String value = select.get(row -> row.getString(1)); + + if (value == null) { + Optional forceAuthenticationSettings = settings.getRawString(PROPERTY_NAME_FORCE_AUTHENTICATION); + + context.prepareUpsert("INSERT INTO properties" + + "(prop_key, is_empty, text_value, created_at, uuid) " + + "VALUES(?, ?, ?, ?, ?)") + .setString(1, PROPERTY_NAME_FORCE_AUTHENTICATION) + .setBoolean(2, false) + .setString(3, forceAuthenticationSettings.orElse("false")) + .setLong(4, system2.now()) + .setString(5, uuidFactory.create()) + .execute() + .commit(); + } + } +} diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchemaTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchemaTest.java index ec620433777..1457e34ef8b 100644 --- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchemaTest.java +++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchemaTest.java @@ -28,7 +28,6 @@ import java.util.stream.Stream; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.sonar.api.SonarRuntime; import org.sonar.api.utils.System2; import org.sonar.api.utils.Version; @@ -45,9 +44,6 @@ public class PopulateInitialSchemaTest { private static final long NOW = 1_500L; - @Rule - public ExpectedException expectedException = ExpectedException.none(); - private final Random random = new Random(); private final Version version = Version.create(1 + random.nextInt(10), 1 + random.nextInt(10), random.nextInt(10)); private UuidFactory uuidFactory = UuidFactoryFast.getInstance(); @@ -77,6 +73,7 @@ public class PopulateInitialSchemaTest { String orgUuid = verifyDefaultOrganization(userGroupId, qgUuid); verifyOrgQualityGate(orgUuid, qgUuid); verifyInternalProperties(orgUuid); + verifyProperties(); verifyRolesOfAdminsGroup(); verifyRolesOfUsersGroup(); verifyRolesOfAnyone(); @@ -100,20 +97,23 @@ public class PopulateInitialSchemaTest { "created_at as \"CREATED_AT\", " + "updated_at as \"UPDATED_AT\" " + "from users where login='admin'"); - assertThat(cols.get("LOGIN")).isEqualTo("admin"); - assertThat(cols.get("NAME")).isEqualTo("Administrator"); + + assertThat(cols) + .containsEntry("LOGIN", "admin") + .containsEntry("NAME", "Administrator") + .containsEntry("EXTERNAL_ID", "admin") + .containsEntry("EXTERNAL_LOGIN", "admin") + .containsEntry("EXT_IDENT_PROVIDER", "sonarqube") + .containsEntry("USER_LOCAL", true) + .containsEntry("CRYPTED_PASSWORD", "$2a$12$uCkkXmhW5ThVK8mpBvnXOOJRLd64LJeHTeCkSuB3lfaR2N0AYBaSi") + .containsEntry("HASH_METHOD", "BCRYPT") + .containsEntry("IS_ROOT", false) + .containsEntry("ONBOARDED", true) + .containsEntry("CREATED_AT", NOW) + .containsEntry("UPDATED_AT", NOW); + assertThat(cols.get("EMAIL")).isNull(); - assertThat(cols.get("EXTERNAL_ID")).isEqualTo("admin"); - assertThat(cols.get("EXTERNAL_LOGIN")).isEqualTo("admin"); - assertThat(cols.get("EXT_IDENT_PROVIDER")).isEqualTo("sonarqube"); - assertThat(cols.get("USER_LOCAL")).isEqualTo(true); - assertThat(cols.get("CRYPTED_PASSWORD")).isEqualTo("$2a$12$uCkkXmhW5ThVK8mpBvnXOOJRLd64LJeHTeCkSuB3lfaR2N0AYBaSi"); assertThat(cols.get("SALT")).isNull(); - assertThat(cols.get("HASH_METHOD")).isEqualTo("BCRYPT"); - assertThat(cols.get("IS_ROOT")).isEqualTo(false); - assertThat(cols.get("ONBOARDED")).isEqualTo(true); - assertThat(cols.get("CREATED_AT")).isEqualTo(NOW); - assertThat(cols.get("UPDATED_AT")).isEqualTo(NOW); } private long verifyGroup(String expectedName, String expectedDescription) { @@ -169,15 +169,17 @@ public class PopulateInitialSchemaTest { assertThat(rows).hasSize(1); Map row = rows.get(0); - assertThat(row.get("KEE")).isEqualTo("default-organization"); - assertThat(row.get("NAME")).isEqualTo("Default Organization"); - assertThat(row.get("GUARDED")).isEqualTo(true); - assertThat(row.get("PRIVATE")).isEqualTo(false); - assertThat(row.get("GROUP_ID")).isEqualTo(userGroupId); - assertThat(row.get("QG_UUID")).isEqualTo(defaultQQUuid); - assertThat(row.get("SUBSCRIPTION")).isEqualTo("SONARQUBE"); - assertThat(row.get("CREATED_AT")).isEqualTo(NOW); - assertThat(row.get("UPDATED_AT")).isEqualTo(NOW); + + assertThat(row) + .containsEntry("KEE", "default-organization") + .containsEntry("NAME", "Default Organization") + .containsEntry("GUARDED", true) + .containsEntry("PRIVATE", false) + .containsEntry("GROUP_ID", userGroupId) + .containsEntry("QG_UUID", defaultQQUuid) + .containsEntry("SUBSCRIPTION", "SONARQUBE") + .containsEntry("CREATED_AT", NOW) + .containsEntry("UPDATED_AT", NOW); return (String) row.get("UUID"); } @@ -191,8 +193,9 @@ public class PopulateInitialSchemaTest { Map row = rows.get(0); assertThat(row.get("UUID")).isNotNull(); - assertThat(row.get("ORG")).isEqualTo(orgUuid); - assertThat(row.get("QG")).isEqualTo(qgUuid); + assertThat(row) + .containsEntry("ORG", orgUuid) + .containsEntry("QG", qgUuid); } private void verifyInternalProperties(String orgUuid) { @@ -212,10 +215,33 @@ public class PopulateInitialSchemaTest { private static void verifyInternalProperty(Map> rowsByKey, String key, String val) { Map row = rowsByKey.get(key); - assertThat(row.get("KEE")).isEqualTo(key); - assertThat(row.get("EMPTY")).isEqualTo(false); - assertThat(row.get("VAL")).isEqualTo(val); - assertThat(row.get("CREATED_AT")).isEqualTo(NOW); + assertThat(row) + .containsEntry("KEE", key) + .containsEntry("EMPTY", false) + .containsEntry("VAL", val) + .containsEntry("CREATED_AT", NOW); + } + + private void verifyProperties() { + List> rows = db.select("select " + + "prop_key as \"PROP_KEY\", " + + "is_empty as \"EMPTY\", " + + "text_value as \"VAL\"," + + "created_at as \"CREATED_AT\" " + + " from properties"); + assertThat(rows).hasSize(1); + + Map> rowsByKey = rows.stream().collect(MoreCollectors.uniqueIndex(t -> (String) t.get("PROP_KEY"))); + verifyProperty(rowsByKey, "sonar.forceAuthentication", "true"); + } + + private static void verifyProperty(Map> rowsByKey, String key, String val) { + Map row = rowsByKey.get(key); + assertThat(row) + .containsEntry("PROP_KEY", key) + .containsEntry("EMPTY", false) + .containsEntry("VAL", val) + .containsEntry("CREATED_AT", NOW); } private void verifyRolesOfAdminsGroup() { diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v86/SetForceAuthenticationSettingsTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v86/SetForceAuthenticationSettingsTest.java new file mode 100644 index 00000000000..b0b054f2cf9 --- /dev/null +++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v86/SetForceAuthenticationSettingsTest.java @@ -0,0 +1,120 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 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.server.platform.db.migration.version.v86; + +import java.sql.SQLException; +import java.util.Optional; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.api.config.internal.Settings; +import org.sonar.api.impl.utils.TestSystem2; +import org.sonar.api.utils.System2; +import org.sonar.core.util.UuidFactoryFast; +import org.sonar.db.CoreDbTester; +import org.sonar.server.platform.db.migration.step.DataChange; + +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.when; + +public class SetForceAuthenticationSettingsTest { + + private static final long NOW = 100_000_000_000L; + + @Rule + public CoreDbTester db = CoreDbTester.createForSchema(SetForceAuthenticationSettingsTest.class, "schema.sql"); + + private final System2 system2 = new TestSystem2().setNow(NOW); + + private final Settings settingsMock = mock(Settings.class); + + private final DataChange underTest = new SetForceAuthenticationSettings(db.database(), system2, UuidFactoryFast.getInstance(), settingsMock); + + @Test + public void insert_force_auth_property_based_on_settings_when_false() throws SQLException { + when(settingsMock.getRawString(any())).thenReturn(Optional.of("false")); + underTest.execute(); + + assertThatForceAuthenticationEquals("false"); + } + + @Test + public void insert_force_auth_property_based_on_settings_when_true() throws SQLException { + when(settingsMock.getRawString(any())).thenReturn(Optional.of("true")); + underTest.execute(); + + assertThatForceAuthenticationEquals("true"); + } + + @Test + public void insert_force_auth_property_based_on_settings_when_empty() throws SQLException { + when(settingsMock.getRawString(any())).thenReturn(Optional.empty()); + underTest.execute(); + + assertThatForceAuthenticationEquals("false"); + } + + @Test + public void insert_force_auth_property_if_row_not_exists() throws SQLException { + underTest.execute(); + + assertThatForceAuthenticationEquals("false"); + } + + @Test + public void migration_is_reentrant() throws SQLException { + underTest.execute(); + // re-entrant + underTest.execute(); + + assertThatForceAuthenticationEquals("false"); + } + + @Test + public void do_nothing_if_force_auth_property_exists_with_value_false() throws SQLException { + insertProperty("uuid-1", "sonar.forceAuthentication", "false"); + underTest.execute(); + + assertThatForceAuthenticationEquals("false"); + } + + @Test + public void do_nothing_if_force_auth_property_exists_with_value_true() throws SQLException { + insertProperty("uuid-1", "sonar.forceAuthentication", "true"); + underTest.execute(); + + assertThatForceAuthenticationEquals("true"); + } + + private void assertThatForceAuthenticationEquals(String s) { + assertThat(db.selectFirst("select p.text_value from properties p where p.prop_key = 'sonar.forceAuthentication'")) + .containsEntry("TEXT_VALUE", s); + } + + private void insertProperty(String uuid, String key, String textValue) { + db.executeInsert("properties", + "uuid", uuid, + "prop_key", key, + "is_empty", false, + "text_value", textValue, + "created_at", NOW); + } +} diff --git a/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v86/SetForceAuthenticationSettingsTest/schema.sql b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v86/SetForceAuthenticationSettingsTest/schema.sql new file mode 100644 index 00000000000..dfe931f54d1 --- /dev/null +++ b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v86/SetForceAuthenticationSettingsTest/schema.sql @@ -0,0 +1,12 @@ +CREATE TABLE "PROPERTIES"( + "PROP_KEY" VARCHAR(512) NOT NULL, + "IS_EMPTY" BOOLEAN NOT NULL, + "TEXT_VALUE" VARCHAR(4000), + "CLOB_VALUE" CLOB, + "CREATED_AT" BIGINT NOT NULL, + "COMPONENT_UUID" VARCHAR(40), + "UUID" VARCHAR(40) NOT NULL, + "USER_UUID" VARCHAR(255) +); +ALTER TABLE "PROPERTIES" ADD CONSTRAINT "PK_PROPERTIES" PRIMARY KEY("UUID"); +CREATE INDEX "PROPERTIES_KEY" ON "PROPERTIES"("PROP_KEY"); diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserSessionInitializer.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserSessionInitializer.java index af4f821dbec..a6b157d8e6d 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserSessionInitializer.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserSessionInitializer.java @@ -34,6 +34,7 @@ import org.sonar.server.user.UserSession; import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED; import static org.apache.commons.lang.StringUtils.defaultString; +import static org.sonar.api.CoreProperties.CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE; import static org.sonar.api.CoreProperties.CORE_FORCE_AUTHENTICATION_PROPERTY; import static org.sonar.api.web.ServletFilter.UrlPattern.Builder.staticResourcePatterns; import static org.sonar.server.authentication.AuthenticationError.handleAuthenticationError; @@ -115,7 +116,7 @@ public class UserSessionInitializer { private void loadUserSession(HttpServletRequest request, HttpServletResponse response, boolean urlSupportsSystemPasscode) { UserSession session = requestAuthenticator.authenticate(request, response); - if (!session.isLoggedIn() && !urlSupportsSystemPasscode && config.getBoolean(CORE_FORCE_AUTHENTICATION_PROPERTY).orElse(false)) { + if (!session.isLoggedIn() && !urlSupportsSystemPasscode && config.getBoolean(CORE_FORCE_AUTHENTICATION_PROPERTY).orElse(CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE)) { // authentication is required throw AuthenticationException.newBuilder() .setSource(Source.local(AuthenticationEvent.Method.BASIC)) diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserSessionInitializerTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserSessionInitializerTest.java index 5fe5ac5008f..e7cdf8c5fc0 100644 --- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserSessionInitializerTest.java +++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserSessionInitializerTest.java @@ -32,6 +32,7 @@ import org.sonar.api.utils.System2; import org.sonar.db.DbTester; import org.sonar.server.authentication.event.AuthenticationEvent; import org.sonar.server.authentication.event.AuthenticationEvent.Method; +import org.sonar.server.authentication.event.AuthenticationEvent.Provider; import org.sonar.server.authentication.event.AuthenticationEvent.Source; import org.sonar.server.authentication.event.AuthenticationException; import org.sonar.server.tester.AnonymousMockUserSession; @@ -41,11 +42,12 @@ import org.sonar.server.user.UserSession; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; public class UserSessionInitializerTest { @@ -111,13 +113,12 @@ public class UserSessionInitializerTest { ArgumentCaptor exceptionArgumentCaptor = ArgumentCaptor.forClass(AuthenticationException.class); when(threadLocalSession.isLoggedIn()).thenReturn(false); when(authenticator.authenticate(request, response)).thenReturn(new AnonymousMockUserSession()); - settings.setProperty("sonar.forceAuthentication", true); assertThat(underTest.initUserSession(request, response)).isTrue(); - verifyZeroInteractions(response); + verifyNoMoreInteractions(response); verify(authenticationEvent).loginFailure(eq(request), exceptionArgumentCaptor.capture()); - verifyZeroInteractions(threadLocalSession); + verifyNoMoreInteractions(threadLocalSession); AuthenticationException authenticationException = exceptionArgumentCaptor.getValue(); assertThat(authenticationException.getSource()).isEqualTo(Source.local(Method.BASIC)); assertThat(authenticationException.getLogin()).isNull(); @@ -125,6 +126,17 @@ public class UserSessionInitializerTest { assertThat(authenticationException.getPublicMessage()).isNull(); } + @Test + public void does_not_return_code_401_when_not_authenticated_and_with_force_authentication_off() { + when(threadLocalSession.isLoggedIn()).thenReturn(false); + when(authenticator.authenticate(request, response)).thenReturn(new AnonymousMockUserSession()); + settings.setProperty("sonar.forceAuthentication", false); + + assertThat(underTest.initUserSession(request, response)).isTrue(); + + verifyNoMoreInteractions(response); + } + @Test public void return_401_and_stop_on_ws() { when(request.getRequestURI()).thenReturn("/api/issues"); @@ -135,7 +147,7 @@ public class UserSessionInitializerTest { verify(response).setStatus(401); verify(authenticationEvent).loginFailure(request, authenticationException); - verifyZeroInteractions(threadLocalSession); + verifyNoMoreInteractions(threadLocalSession); } @Test @@ -147,7 +159,7 @@ public class UserSessionInitializerTest { assertThat(underTest.initUserSession(request, response)).isFalse(); verify(response).setStatus(401); - verifyZeroInteractions(threadLocalSession); + verifyNoMoreInteractions(threadLocalSession); } @Test @@ -184,7 +196,7 @@ public class UserSessionInitializerTest { assertThat(underTest.initUserSession(request, response)).isTrue(); - verifyZeroInteractions(threadLocalSession, authenticator); + verifyNoMoreInteractions(threadLocalSession, authenticator); reset(threadLocalSession, authenticator); } diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/platform/monitoring/StandaloneSystemSection.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/platform/monitoring/StandaloneSystemSection.java index 869cb8eb1c3..0ce16ba4de9 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/platform/monitoring/StandaloneSystemSection.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/platform/monitoring/StandaloneSystemSection.java @@ -36,6 +36,7 @@ import org.sonar.server.platform.DockerSupport; import org.sonar.server.platform.OfficialDistribution; import org.sonar.server.user.SecurityRealmFactory; +import static org.sonar.api.CoreProperties.CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE; import static org.sonar.process.ProcessProperties.Property.PATH_DATA; import static org.sonar.process.ProcessProperties.Property.PATH_HOME; import static org.sonar.process.ProcessProperties.Property.PATH_TEMP; @@ -104,7 +105,7 @@ public class StandaloneSystemSection extends BaseSectionMBean implements SystemS } private boolean getForceAuthentication() { - return config.getBoolean(CoreProperties.CORE_FORCE_AUTHENTICATION_PROPERTY).orElse(false); + return config.getBoolean(CoreProperties.CORE_FORCE_AUTHENTICATION_PROPERTY).orElse(CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE); } @Override diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/platform/monitoring/cluster/GlobalSystemSection.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/platform/monitoring/cluster/GlobalSystemSection.java index fc52bc1b527..0b90fee89da 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/platform/monitoring/cluster/GlobalSystemSection.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/platform/monitoring/cluster/GlobalSystemSection.java @@ -37,6 +37,7 @@ import org.sonar.server.authentication.IdentityProviderRepository; import org.sonar.server.platform.DockerSupport; import org.sonar.server.user.SecurityRealmFactory; +import static org.sonar.api.CoreProperties.CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE; import static org.sonar.process.systeminfo.SystemInfoUtils.setAttribute; @ServerSide @@ -91,7 +92,7 @@ public class GlobalSystemSection implements SystemInfoSection, Global { } private boolean getForceAuthentication() { - return config.getBoolean(CoreProperties.CORE_FORCE_AUTHENTICATION_PROPERTY).orElse(false); + return config.getBoolean(CoreProperties.CORE_FORCE_AUTHENTICATION_PROPERTY).orElse(CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE); } private static void addIfNotEmpty(ProtobufSystemInfo.Section.Builder protobuf, String key, @Nullable List values) { diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/platform/monitoring/StandaloneSystemSectionTest.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/platform/monitoring/StandaloneSystemSectionTest.java index a5bdc13d06d..2c9f613c4ac 100644 --- a/server/sonar-webserver-core/src/test/java/org/sonar/server/platform/monitoring/StandaloneSystemSectionTest.java +++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/platform/monitoring/StandaloneSystemSectionTest.java @@ -26,6 +26,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; +import org.sonar.api.CoreProperties; import org.sonar.api.config.internal.MapSettings; import org.sonar.api.platform.Server; import org.sonar.api.security.SecurityRealm; @@ -154,7 +155,20 @@ public class StandaloneSystemSectionTest { @Test public void return_nb_of_processors() { ProtobufSystemInfo.Section protobuf = underTest.toProtobuf(); - assertThat(attribute(protobuf, "Processors").getLongValue()).isGreaterThan(0); + assertThat(attribute(protobuf, "Processors").getLongValue()).isPositive(); + } + + @Test + public void get_force_authentication_defaults_to_true() { + ProtobufSystemInfo.Section protobuf = underTest.toProtobuf(); + assertThatAttributeIs(protobuf, "Force authentication", true); + } + + @Test + public void get_force_authentication() { + settings.setProperty(CoreProperties.CORE_FORCE_AUTHENTICATION_PROPERTY, false); + ProtobufSystemInfo.Section protobuf = underTest.toProtobuf(); + assertThatAttributeIs(protobuf, "Force authentication", false); } @Test diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/platform/monitoring/cluster/GlobalSystemSectionTest.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/platform/monitoring/cluster/GlobalSystemSectionTest.java index d3a1ff6b768..49fbff69fd6 100644 --- a/server/sonar-webserver-core/src/test/java/org/sonar/server/platform/monitoring/cluster/GlobalSystemSectionTest.java +++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/platform/monitoring/cluster/GlobalSystemSectionTest.java @@ -25,6 +25,7 @@ import com.tngtech.java.junit.dataprovider.UseDataProvider; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; +import org.sonar.api.CoreProperties; import org.sonar.api.config.internal.MapSettings; import org.sonar.api.platform.Server; import org.sonar.api.security.SecurityRealm; @@ -118,6 +119,19 @@ public class GlobalSystemSectionTest { assertThatAttributeIs(protobuf, "External identity providers whose users are allowed to sign themselves up", "GitHub"); } + @Test + public void get_force_authentication_defaults_to_true() { + ProtobufSystemInfo.Section protobuf = underTest.toProtobuf(); + assertThatAttributeIs(protobuf, "Force authentication", true); + } + + @Test + public void get_force_authentication() { + settings.setProperty(CoreProperties.CORE_FORCE_AUTHENTICATION_PROPERTY, false); + ProtobufSystemInfo.Section protobuf = underTest.toProtobuf(); + assertThatAttributeIs(protobuf, "Force authentication", false); + } + @Test @UseDataProvider("trueOrFalse") public void get_docker_flag(boolean flag) { diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/authentication/ws/ValidateAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/authentication/ws/ValidateAction.java index e967996175c..061510b86e2 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/authentication/ws/ValidateAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/authentication/ws/ValidateAction.java @@ -39,6 +39,7 @@ import org.sonar.server.authentication.event.AuthenticationException; import org.sonar.server.ws.ServletFilterHandler; import org.sonarqube.ws.MediaTypes; +import static org.sonar.api.CoreProperties.CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE; import static org.sonar.api.CoreProperties.CORE_FORCE_AUTHENTICATION_PROPERTY; import static org.sonar.server.authentication.ws.AuthenticationWs.AUTHENTICATION_CONTROLLER; @@ -96,7 +97,7 @@ public class ValidateAction extends ServletFilter implements AuthenticationWsAct if (user.isPresent()) { return true; } - return !config.getBoolean(CORE_FORCE_AUTHENTICATION_PROPERTY).orElse(false); + return !config.getBoolean(CORE_FORCE_AUTHENTICATION_PROPERTY).orElse(CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE); } catch (AuthenticationException e) { return false; } diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/authentication/ws/ValidateActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/authentication/ws/ValidateActionTest.java index 368b518095d..fedb5224bd2 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/authentication/ws/ValidateActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/authentication/ws/ValidateActionTest.java @@ -125,6 +125,17 @@ public class ValidateActionTest { JsonAssert.assertJson(stringWriter.toString()).isSimilarTo("{\"valid\":false}"); } + @Test + public void return_false_when_no_jwt_nor_basic_auth_and_force_authentication_fallback_to_default() throws Exception { + when(jwtHttpHandler.validateToken(request, response)).thenReturn(Optional.empty()); + when(basicAuthentication.authenticate(request)).thenReturn(Optional.empty()); + + underTest.doFilter(request, response, chain); + + verify(response).setContentType(MediaTypes.JSON); + JsonAssert.assertJson(stringWriter.toString()).isSimilarTo("{\"valid\":false}"); + } + @Test public void return_false_when_jwt_throws_unauthorized_exception() throws Exception { doThrow(AuthenticationException.class).when(jwtHttpHandler).validateToken(request, response); diff --git a/sonar-core/src/main/java/org/sonar/core/config/SecurityProperties.java b/sonar-core/src/main/java/org/sonar/core/config/SecurityProperties.java index a9e93b8a5a2..17f898d8512 100644 --- a/sonar-core/src/main/java/org/sonar/core/config/SecurityProperties.java +++ b/sonar-core/src/main/java/org/sonar/core/config/SecurityProperties.java @@ -39,7 +39,8 @@ class SecurityProperties { .name("Force user authentication") .description( "Forcing user authentication prevents anonymous users from accessing the SonarQube UI, or project data via the Web API. " - + "Some specific read-only Web APIs, including those required to prompt authentication, are still available anonymously.") + + "Some specific read-only Web APIs, including those required to prompt authentication, are still available anonymously." + + "
Disabling this setting can expose the instance to security risks.") .type(PropertyType.BOOLEAN) .category(CoreProperties.CATEGORY_SECURITY) .build()); diff --git a/sonar-core/src/test/java/org/sonar/core/config/SecurityPropertiesTest.java b/sonar-core/src/test/java/org/sonar/core/config/SecurityPropertiesTest.java new file mode 100644 index 00000000000..21c60bcb075 --- /dev/null +++ b/sonar-core/src/test/java/org/sonar/core/config/SecurityPropertiesTest.java @@ -0,0 +1,42 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 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.core.config; + +import java.util.Optional; +import org.junit.Test; +import org.sonar.api.config.PropertyDefinition; + +import static org.assertj.core.api.Assertions.assertThat; + +public class SecurityPropertiesTest { + + @Test + public void creates_properties() { + assertThat(SecurityProperties.all()).isNotEmpty(); + Optional propertyDefinition = SecurityProperties.all().stream() + .filter(d -> d.key().equals("sonar.forceAuthentication")).findFirst(); + assertThat(propertyDefinition) + .isNotEmpty() + .get() + .extracting(PropertyDefinition::defaultValue) + .isEqualTo("true"); + } + +} diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/CoreProperties.java b/sonar-plugin-api/src/main/java/org/sonar/api/CoreProperties.java index 3b14bca939e..fa5ac957355 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/CoreProperties.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/CoreProperties.java @@ -197,7 +197,7 @@ public interface CoreProperties { /* Sonar Core */ String CORE_FORCE_AUTHENTICATION_PROPERTY = "sonar.forceAuthentication"; - boolean CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE = false; + boolean CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE = true; /** * @deprecated since 2.14. See http://jira.sonarsource.com/browse/SONAR-3153. Replaced by {@link #CORE_AUTHENTICATOR_REALM}. -- 2.39.5