From c7f7e91276a0012cfbf5ba918e22f8da9c78bd5c Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 5 Jan 2017 10:29:17 +0100 Subject: [PATCH] SONAR-8236 License type setting should be visble only when authenticated Permissions should be correctly checked even when the license doesn't ends with ".license.secured" --- .../ws/SettingsPermissionPredicates.java | 24 ++++++++++++------- .../setting/ws/ListDefinitionsActionTest.java | 21 +++++++++------- .../server/setting/ws/ValuesActionTest.java | 12 ++++++---- 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsPermissionPredicates.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsPermissionPredicates.java index 9e4df29db08..73b7bb40a6e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsPermissionPredicates.java +++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsPermissionPredicates.java @@ -21,11 +21,13 @@ package org.sonar.server.setting.ws; import java.util.Optional; import java.util.function.Predicate; +import javax.annotation.Nullable; import org.sonar.api.config.PropertyDefinition; import org.sonar.core.permission.GlobalPermissions; import org.sonar.db.component.ComponentDto; import org.sonar.server.user.UserSession; +import static org.sonar.api.PropertyType.LICENSE; import static org.sonar.api.web.UserRole.ADMIN; public class SettingsPermissionPredicates { @@ -41,24 +43,30 @@ public class SettingsPermissionPredicates { } Predicate isSettingVisible(Optional component) { - return setting -> isVisible(setting.getKey(), component); + return setting -> isVisible(setting.getKey(), setting.getDefinition(), component); } Predicate isDefinitionVisible(Optional component) { - return propertyDefinition -> isVisible(propertyDefinition.key(), component); + return propertyDefinition -> isVisible(propertyDefinition.key(), propertyDefinition, component); } - boolean isVisible(String settingKey, Optional component) { - return !settingKey.endsWith(SECURED_SUFFIX) - || hasAdminPermission(component) - || (isLicenseRelated(settingKey) && userSession.isLoggedIn()); + private boolean isVisible(String key, @Nullable PropertyDefinition definition, Optional component) { + return verifySecuredSetting(key, definition, component) && (verifyLicenseSetting(key, definition)); + } + + private boolean verifySecuredSetting(String key, @Nullable PropertyDefinition definition, Optional component) { + return isLicense(key, definition) || (!key.endsWith(SECURED_SUFFIX) || hasAdminPermission(component)); } private boolean hasAdminPermission(Optional component) { return component.isPresent() ? userSession.hasComponentUuidPermission(ADMIN, component.get().uuid()) : userSession.hasPermission(GlobalPermissions.SYSTEM_ADMIN); } - private static boolean isLicenseRelated(String settingKey) { - return settingKey.endsWith(LICENSE_SUFFIX) || settingKey.endsWith(LICENSE_HASH_SUFFIX); + private boolean verifyLicenseSetting(String key, @Nullable PropertyDefinition definition) { + return !isLicense(key, definition) || userSession.isLoggedIn(); + } + + private static boolean isLicense(String key, @Nullable PropertyDefinition definition) { + return key.endsWith(LICENSE_SUFFIX) || key.endsWith(LICENSE_HASH_SUFFIX) || (definition != null && definition.type() == LICENSE); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ListDefinitionsActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ListDefinitionsActionTest.java index 26c91847ab5..077d9f09e58 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ListDefinitionsActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ListDefinitionsActionTest.java @@ -22,7 +22,6 @@ package org.sonar.server.setting.ws; import java.io.IOException; import javax.annotation.Nullable; -import org.assertj.core.groups.Tuple; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -49,6 +48,7 @@ import org.sonarqube.ws.Settings.ListDefinitionsWsResponse; import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.groups.Tuple.tuple; import static org.sonar.api.resources.Qualifiers.MODULE; import static org.sonar.api.resources.Qualifiers.PROJECT; import static org.sonar.api.web.UserRole.ADMIN; @@ -237,7 +237,7 @@ public class ListDefinitionsActionTest { ListDefinitionsWsResponse result = executeRequest(); assertThat(result.getDefinitionsList()).hasSize(1); - assertThat(result.getDefinitions(0).getFieldsList()).extracting(Settings.Field::getKey, Settings.Field::getType).containsOnly(Tuple.tuple("license", LICENSE)); + assertThat(result.getDefinitions(0).getFieldsList()).extracting(Settings.Field::getKey, Settings.Field::getType).containsOnly(tuple("license", LICENSE)); } @Test @@ -304,19 +304,23 @@ public class ListDefinitionsActionTest { @Test public void return_license_type() throws Exception { setUserAsSystemAdmin(); - propertyDefinitions.addComponent(PropertyDefinition.builder("license").type(PropertyType.LICENSE).build()); + propertyDefinitions.addComponents(asList( + PropertyDefinition.builder("plugin.license.secured").type(PropertyType.LICENSE).build(), + PropertyDefinition.builder("commercial.plugin").type(PropertyType.LICENSE).build())); ListDefinitionsWsResponse result = executeRequest(); - assertThat(result.getDefinitionsList()).extracting(Settings.Definition::getKey, Settings.Definition::getType).containsOnly(Tuple.tuple("license", LICENSE)); + assertThat(result.getDefinitionsList()).extracting(Settings.Definition::getKey, Settings.Definition::getType) + .containsOnly(tuple("plugin.license.secured", LICENSE), tuple("commercial.plugin", LICENSE)); } @Test - public void does_not_returned_secured_settings_when_not_authenticated() throws Exception { + public void does_not_returned_secured_and_license_settings_when_not_authenticated() throws Exception { propertyDefinitions.addComponents(asList( PropertyDefinition.builder("foo").build(), PropertyDefinition.builder("secret.secured").build(), - PropertyDefinition.builder("plugin.license.secured").type(PropertyType.LICENSE).build())); + PropertyDefinition.builder("plugin.license.secured").type(PropertyType.LICENSE).build(), + PropertyDefinition.builder("commercial.plugin").type(PropertyType.LICENSE).build())); ListDefinitionsWsResponse result = executeRequest(); @@ -329,11 +333,12 @@ public class ListDefinitionsActionTest { propertyDefinitions.addComponents(asList( PropertyDefinition.builder("foo").build(), PropertyDefinition.builder("secret.secured").build(), - PropertyDefinition.builder("plugin.license.secured").type(PropertyType.LICENSE).build())); + PropertyDefinition.builder("plugin.license.secured").type(PropertyType.LICENSE).build(), + PropertyDefinition.builder("commercial.plugin").type(PropertyType.LICENSE).build())); ListDefinitionsWsResponse result = executeRequest(); - assertThat(result.getDefinitionsList()).extracting(Settings.Definition::getKey).containsOnly("foo", "plugin.license.secured"); + assertThat(result.getDefinitionsList()).extracting(Settings.Definition::getKey).containsOnly("foo", "plugin.license.secured", "commercial.plugin"); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ValuesActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ValuesActionTest.java index 5779cef1281..a6591d4334a 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ValuesActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ValuesActionTest.java @@ -453,17 +453,19 @@ public class ValuesActionTest { } @Test - public void does_not_returned_secured_settings_when_not_authenticated() throws Exception { + public void does_not_returned_secured_and_license_settings_when_not_authenticated() throws Exception { definitions.addComponents(asList( PropertyDefinition.builder("foo").build(), PropertyDefinition.builder("secret.secured").build(), + PropertyDefinition.builder("commercial.plugin").type(LICENSE).build(), PropertyDefinition.builder("plugin.license.secured").type(LICENSE).build())); propertyDb.insertProperties( newGlobalPropertyDto().setKey("foo").setValue("one"), newGlobalPropertyDto().setKey("secret.secured").setValue("password"), + newGlobalPropertyDto().setKey("commercial.plugin").setValue("ABCD"), newGlobalPropertyDto().setKey("plugin.license.secured").setValue("ABCD")); - ValuesWsResponse result = executeRequestForGlobalProperties("foo", "secret.secured", "plugin.license.secured"); + ValuesWsResponse result = executeRequestForGlobalProperties("foo", "secret.secured", "commercial.plugin", "plugin.license.secured"); assertThat(result.getSettingsList()).extracting(Settings.Setting::getKey).containsOnly("foo"); } @@ -474,16 +476,18 @@ public class ValuesActionTest { definitions.addComponents(asList( PropertyDefinition.builder("foo").build(), PropertyDefinition.builder("secret.secured").build(), + PropertyDefinition.builder("commercial.plugin").type(LICENSE).build(), PropertyDefinition.builder("plugin.license.secured").type(LICENSE).build())); propertyDb.insertProperties( newGlobalPropertyDto().setKey("foo").setValue("one"), newGlobalPropertyDto().setKey("secret.secured").setValue("password"), + newGlobalPropertyDto().setKey("commercial.plugin").setValue("ABCD"), newGlobalPropertyDto().setKey("plugin.license.secured").setValue("ABCD"), newGlobalPropertyDto().setKey("plugin.licenseHash.secured").setValue("987654321")); - ValuesWsResponse result = executeRequestForGlobalProperties("foo", "secret.secured", "plugin.license.secured", "plugin.licenseHash.secured"); + ValuesWsResponse result = executeRequestForGlobalProperties("foo", "secret.secured", "commercial.plugin", "plugin.license.secured", "plugin.licenseHash.secured"); - assertThat(result.getSettingsList()).extracting(Settings.Setting::getKey).containsOnly("foo", "plugin.license.secured", "plugin.licenseHash.secured"); + assertThat(result.getSettingsList()).extracting(Settings.Setting::getKey).containsOnly("foo", "commercial.plugin", "plugin.license.secured", "plugin.licenseHash.secured"); } @Test -- 2.39.5