SONAR-21795 Fix SSF-565

diff --git a/server/sonar-web/src/main/js/apps/settings/components/almIntegration/GithubForm.tsx b/server/sonar-web/src/main/js/apps/settings/components/almIntegration/GithubForm.tsx
index 623c3043015ac99b5ea664dbfb4fcd49ae66990f..e48a6271841e78136797e49932eddfd048c3a17e 100644 (file)
--- a/server/sonar-web/src/main/js/apps/settings/components/almIntegration/GithubForm.tsx
+++ b/server/sonar-web/src/main/js/apps/settings/components/almIntegration/GithubForm.tsx
@@ -67,6 +67,9 @@ export default function GithubForm(props: GithubFormProps) {
             {translate('settings.almintegration.form.url.github.help2')}
             <br />
             <em>https://api.github.com/</em>
+            <br />
+            <br />
+            {translate('settings.almintegration.form.url.github.private_key_warning')}
           </>
         }
         id="url.github"

diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/almsettings/ws/UpdateGithubActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/almsettings/ws/UpdateGithubActionIT.java
index cb8db09c57afe9774e429a8d07538e94b859ed81..560119cdba50340217d3d0f8b5701e5f753c2042 100644 (file)
--- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/almsettings/ws/UpdateGithubActionIT.java
+++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/almsettings/ws/UpdateGithubActionIT.java
@@ -123,14 +123,14 @@ public class UpdateGithubActionIT {
   }
 
   @Test
-  public void update_without_private_key_nor_client_secret() {
+  public void update_without_client_secret() {
     buildTestRequestWithoutSecrets().execute();
 
     assertThat(db.getDbClient().almSettingDao().selectAll(db.getSession()))
       .extracting(AlmSettingDto::getKey, AlmSettingDto::getUrl, AlmSettingDto::getAppId,
         s -> s.getDecryptedPrivateKey(encryption), AlmSettingDto::getClientId, s -> s.getDecryptedClientSecret(encryption))
       .containsOnly(tuple(almSettingDto.getKey(), "https://github.enterprise-unicorn.com", "54321",
-        almSettingDto.getDecryptedPrivateKey(encryption), "client_1234", almSettingDto.getDecryptedClientSecret(encryption)));
+        "10987654321", "client_1234", almSettingDto.getDecryptedClientSecret(encryption)));
   }
 
 
@@ -139,7 +139,8 @@ public class UpdateGithubActionIT {
       .setParam("key", almSettingDto.getKey())
       .setParam("url", "https://github.enterprise-unicorn.com/")
       .setParam("appId", "54321")
-      .setParam("clientId", "client_1234");
+      .setParam("clientId", "client_1234")
+      .setParam("privateKey", "10987654321");
   }
 
   @Test
@@ -166,6 +167,34 @@ public class UpdateGithubActionIT {
       .hasMessageContaining(format("An DevOps Platform setting with key '%s' already exists", almSetting2.getKey()));
   }
 
+  @Test
+  public void update_without_url_changes_does_not_need_private_key() {
+    TestRequest request = ws.newRequest()
+      .setParam("key", almSettingDto.getKey())
+      .setParam("url", almSettingDto.getUrl())
+      .setParam("appId", "54321")
+      .setParam("clientId", "client_1234");
+
+    request.execute();
+
+    assertThat(db.getDbClient().almSettingDao().selectAll(db.getSession()))
+      .extracting(AlmSettingDto::getKey, AlmSettingDto::getUrl, AlmSettingDto::getAppId, AlmSettingDto::getClientId)
+      .containsOnly(tuple(almSettingDto.getKey(), almSettingDto.getUrl(), "54321", "client_1234"));
+  }
+
+  @Test
+  public void fail_when_url_updated_without_private_key() {
+    TestRequest request = ws.newRequest()
+      .setParam("key", almSettingDto.getKey())
+      .setParam("url", "https://github.enterprise-unicorn.com")
+      .setParam("appId", "54321")
+      .setParam("clientId", "client_1234");
+
+    assertThatThrownBy(request::execute)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessageContaining("Please provide the Private Key to update the URL.");
+  }
+
   @Test
   public void fail_when_missing_administer_system_permission() {
     UserDto user = db.users().insertUser();
@@ -250,8 +279,7 @@ public class UpdateGithubActionIT {
   public static Object[][] secretParams() {
     return new Object[][] {
       {"webhookSecret"},
-      {"clientSecret"},
-      {"privateKey"}
+      {"clientSecret"}
     };
   }
 

diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/AlmSettingsSupport.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/AlmSettingsSupport.java
index 5d7fd8043b57ad15937a8059d57f46542bda226c..182c3cb11f9e909ce8cca7100aba0f4741b94e9f 100644 (file)
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/AlmSettingsSupport.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/AlmSettingsSupport.java
@@ -20,6 +20,7 @@
 package org.sonar.server.almsettings.ws;
 
 import java.util.regex.Pattern;
+import javax.annotation.Nullable;
 import org.sonar.api.server.ServerSide;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
@@ -34,6 +35,7 @@ import org.sonar.server.user.UserSession;
 import org.sonarqube.ws.AlmSettings;
 
 import static java.lang.String.format;
+import static org.apache.commons.lang.StringUtils.isEmpty;
 import static org.sonar.api.web.UserRole.ADMIN;
 
 @ServerSide
@@ -109,4 +111,10 @@ public class AlmSettingsSupport {
         throw new IllegalStateException(format("Unknown DevOps Platform '%s'", alm.name()));
     }
   }
+
+  public void checkPrivateKeyOnUrlUpdate(AlmSettingDto almSettingDto, String url, @Nullable String privateKey) {
+    if (!url.equals(almSettingDto.getUrl()) && isEmpty(privateKey)) {
+      throw new IllegalArgumentException("Please provide the Private Key to update the URL.");
+    }
+  }
 }

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 7fa7da69c9a2375bf3994d120b65065f05d99907..fd1473d804cedc5759a9ba62d7b1871f801d2a37 100644 (file)
--- 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
@@ -116,13 +116,17 @@ public class UpdateGithubAction implements AlmSettingsWsAction {
   private void doHandle(Request request, DbSession dbSession) {
     String key = request.mandatoryParam(PARAM_KEY);
     String newKey = request.param(PARAM_NEW_KEY);
+
     if (isNotBlank(newKey) && !newKey.equals(key)) {
       almSettingsSupport.checkAlmSettingDoesNotAlreadyExist(dbSession, newKey);
     }
 
     AlmSettingDto almSettingDto = almSettingsSupport.getAlmSetting(dbSession, key);
-
+    String url = request.mandatoryParam(PARAM_URL);
     String privateKey = request.param(PARAM_PRIVATE_KEY);
+
+    almSettingsSupport.checkPrivateKeyOnUrlUpdate(almSettingDto, url, privateKey);
+
     if (isNotBlank(privateKey)) {
       almSettingDto.setPrivateKey(privateKey);
     }

diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
index ab76193c419f03a0eb54cc3341aa3fd7ce015977..8286d2f65aad20f8d6f6d0635cfd8e16862ac25b 100644 (file)
--- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties
+++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
@@ -1460,6 +1460,7 @@ settings.almintegration.form.url.bitbucket.help=Example: {example}
 settings.almintegration.form.url.github=GitHub API URL
 settings.almintegration.form.url.github.help1=Example for GitHub Enterprise:
 settings.almintegration.form.url.github.help2=If using GitHub.com:
+settings.almintegration.form.url.github.private_key_warning=Please make sure to provide the GitHub App private key for updating the URL.
 settings.almintegration.form.url.gitlab=GitLab API URL
 settings.almintegration.form.url.gitlab.help=Provide the GitLab API URL. For example:
 settings.almintegration.form.app_id=GitHub App ID