SONAR-19785 Add required new permissions in the GitHub config check

diff --git a/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubBinding.java b/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubBinding.java
index 779fefbadd53a53d036f2ae13d753e6d5bb3def0..8296839a3e1d65efdbdad67f0f934b206db35a34 100644 (file)
--- a/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubBinding.java
+++ b/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/GithubBinding.java
@@ -150,12 +150,19 @@ public class GithubBinding {
     String emails;
     @SerializedName("contents")
     String contents;
+    @SerializedName("metadata")
+    String metadata;
+    @SerializedName("administration")
+    String administration;
 
-    public Permissions(@Nullable String checks, @Nullable String members, @Nullable String emails, @Nullable String contents) {
+    public Permissions(@Nullable String checks, @Nullable String members, @Nullable String emails, @Nullable String contents, @Nullable String metadata,
+      @Nullable String administration) {
       this.checks = checks;
       this.members = members;
       this.emails = emails;
       this.contents = contents;
+      this.metadata = metadata;
+      this.administration = administration;
     }
 
     public Permissions() {
@@ -164,6 +171,10 @@ public class GithubBinding {
       // http://stackoverflow.com/a/18645370/229031
     }
 
+    public static Builder builder() {
+      return new Builder();
+    }
+
     @CheckForNull
     public String getMembers() {
       return members;
@@ -183,6 +194,63 @@ public class GithubBinding {
     public String getContents() {
       return contents;
     }
+
+    @CheckForNull
+    public String getMetadata() {
+      return metadata;
+    }
+
+    @CheckForNull
+    public String getAdministration() {
+      return administration;
+    }
+
+    public static class Builder {
+      private String checks;
+      private String members;
+      private String emails;
+      private String contents;
+      private String metadata;
+      private String administration;
+
+      private Builder() {
+        // Use static factory method
+      }
+
+      public Builder setChecks(String checks) {
+        this.checks = checks;
+        return this;
+      }
+
+      public Builder setMembers(String members) {
+        this.members = members;
+        return this;
+      }
+
+      public Builder setEmails(String emails) {
+        this.emails = emails;
+        return this;
+      }
+
+      public Builder setContents(String contents) {
+        this.contents = contents;
+        return this;
+      }
+
+      public Builder setMetadata(String metadata) {
+        this.metadata = metadata;
+        return this;
+      }
+
+      public Builder setAdministration(String administration) {
+        this.administration = administration;
+        return this;
+      }
+
+      public GithubBinding.Permissions build() {
+        return new GithubBinding.Permissions(checks, members, emails, contents, metadata, administration);
+      }
+    }
   }
 
   public static class GsonRepositorySearch {

diff --git a/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/config/GithubProvisioningConfigValidator.java b/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/config/GithubProvisioningConfigValidator.java
index efa137121c267ae42a30751a304ff8bc09ced7c5..9ed7e9fed6743fc5a45235cb276772b236cc35b6 100644 (file)
--- a/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/config/GithubProvisioningConfigValidator.java
+++ b/server/sonar-alm-client/src/main/java/org/sonar/alm/client/github/config/GithubProvisioningConfigValidator.java
@@ -39,8 +39,10 @@ import static org.sonar.alm.client.github.config.ConfigCheckResult.InstallationS
 @ComputeEngineSide
 public class GithubProvisioningConfigValidator {
 
-  private static final String MEMBERS_PERMISSION = "Organization permissions -> Members";
-  private static final String EMAILS_PERMISSION = "Account permissions -> Email addresses";
+  private static final String ORG_MEMBERS_PERMISSION = "Organization permissions -> Members (Read-only)";
+  private static final String ACCOUNT_EMAILS_PERMISSION = "Account permissions -> Email addresses (Read-only)";
+  private static final String REPO_ADMIN_PERMISSION = "Repository permissions -> Administration (Read-only)";
+  private static final String REPO_METADATA_PERMISSION = "Repository permissions -> Metadata (Read-only)";
   private static final ConfigStatus INVALID_APP_CONFIG_STATUS = ConfigStatus.failed("The GitHub App configuration is not complete.");
   private static final ConfigStatus INVALID_APP_ID_STATUS = ConfigStatus.failed("GitHub App ID must be a number.");
   private static final ConfigStatus SUSPENDED_INSTALLATION_STATUS = ConfigStatus.failed("Installation suspended");
@@ -117,7 +119,7 @@ public class GithubProvisioningConfigValidator {
 
   private static ConfigStatus jitAppConfigStatus(Permissions permissions) {
     if (permissions.getEmails() == null) {
-      return failedStatus(List.of(EMAILS_PERMISSION));
+      return failedStatus(List.of(ACCOUNT_EMAILS_PERMISSION));
     }
     return ConfigStatus.SUCCESS;
   }
@@ -125,10 +127,16 @@ public class GithubProvisioningConfigValidator {
   private static ConfigStatus autoProvisioningAppConfigStatus(Permissions permissions) {
     List<String> missingPermissions = new ArrayList<>();
     if (permissions.getEmails() == null) {
-      missingPermissions.add(EMAILS_PERMISSION);
+      missingPermissions.add(ACCOUNT_EMAILS_PERMISSION);
     }
     if (permissions.getMembers() == null) {
-      missingPermissions.add(MEMBERS_PERMISSION);
+      missingPermissions.add(ORG_MEMBERS_PERMISSION);
+    }
+    if (permissions.getAdministration() == null) {
+      missingPermissions.add(REPO_ADMIN_PERMISSION);
+    }
+    if (permissions.getMetadata() == null) {
+      missingPermissions.add(REPO_METADATA_PERMISSION);
     }
     if (missingPermissions.isEmpty()) {
       return ConfigStatus.SUCCESS;
@@ -137,7 +145,7 @@ public class GithubProvisioningConfigValidator {
   }
 
   private static ConfigStatus failedStatus(List<String> missingPermissions) {
-    return ConfigStatus.failed("Missing permissions: " + String.join(",", missingPermissions));
+    return ConfigStatus.failed("Missing GitHub permissions: " + String.join(", ", missingPermissions));
   }
 
   private List<InstallationStatus> checkInstallations(GithubAppConfiguration githubAppConfiguration, ApplicationStatus appStatus) {
@@ -158,10 +166,20 @@ public class GithubProvisioningConfigValidator {
   }
 
   private static ConfigStatus autoProvisioningInstallationConfigStatus(Permissions permissions) {
+    List<String> missingPermissions = new ArrayList<>();
     if (permissions.getMembers() == null) {
-      return failedStatus(List.of(MEMBERS_PERMISSION));
+      missingPermissions.add(ORG_MEMBERS_PERMISSION);
     }
-    return ConfigStatus.SUCCESS;
+    if (permissions.getAdministration() == null) {
+      missingPermissions.add(REPO_ADMIN_PERMISSION);
+    }
+    if (permissions.getMetadata() == null) {
+      missingPermissions.add(REPO_METADATA_PERMISSION);
+    }
+    if (missingPermissions.isEmpty()) {
+      return ConfigStatus.SUCCESS;
+    }
+    return failedStatus(missingPermissions);
   }
 
 }

diff --git a/server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/config/GithubProvisioningConfigValidatorTest.java b/server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/config/GithubProvisioningConfigValidatorTest.java
index 2d64e2b4a5ca060b4d127d3d4a9bc4efcda4e122..f3191bc3a929c4e50b5902a1f59cb34ebf367240 100644 (file)
--- a/server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/config/GithubProvisioningConfigValidatorTest.java
+++ b/server/sonar-alm-client/src/test/java/org/sonar/alm/client/github/config/GithubProvisioningConfigValidatorTest.java
@@ -51,9 +51,11 @@ public class GithubProvisioningConfigValidatorTest {
   private static final String APP_FETCHING_FAILED = "Exception while fetching the App.";
   private static final String INVALID_APP_ID_STATUS = "GitHub App ID must be a number.";
   private static final String INCOMPLETE_APP_CONFIG_STATUS = "The GitHub App configuration is not complete.";
-  private static final String MISSING_EMAIL_PERMISSION = "Missing permissions: Account permissions -> Email addresses";
-  private static final String MISSING_MEMBERS_PERMISSION = "Missing permissions: Organization permissions -> Members";
-  private static final String MISSING_EMAILS_AND_MEMBERS_PERMISSION = "Missing permissions: Account permissions -> Email addresses,Organization permissions -> Members";
+  private static final String MISSING_EMAIL_PERMISSION = "Missing GitHub permissions: Account permissions -> Email addresses (Read-only)";
+  private static final String MISSING_ALL_AUTOPROVISIONNING_PERMISSIONS = "Missing GitHub permissions: Organization permissions -> Members (Read-only), "
+    + "Repository permissions -> Administration (Read-only), Repository permissions -> Metadata (Read-only)";
+  private static final String MISSING_ALL_PERMISSIONS = "Missing GitHub permissions: Account permissions -> Email addresses (Read-only), "
+    + "Organization permissions -> Members (Read-only), Repository permissions -> Administration (Read-only), Repository permissions -> Metadata (Read-only)";
   private static final String NO_INSTALLATIONS_STATUS = "The GitHub App is not installed on any organizations or the organization is not white-listed.";
   private static final String SUSPENDED_INSTALLATION = "Installation suspended";
 
@@ -138,41 +140,41 @@ public class GithubProvisioningConfigValidatorTest {
   }
 
   @Test
-  public void checkConfig_whenAppDoesntHaveEmailsPermissions_shouldReturnFailedAppJitCheck() {
+  public void checkConfig_whenAppDoesntHaveAnyPermissions_shouldReturnFailedAppJitCheck() {
     mockGithubConfiguration();
     ArgumentCaptor<GithubAppConfiguration> appConfigurationCaptor = ArgumentCaptor.forClass(GithubAppConfiguration.class);
     GsonApp githubApp = mockGithubApp(appConfigurationCaptor);
 
-    when(githubApp.getPermissions()).thenReturn(new Permissions());
+    when(githubApp.getPermissions()).thenReturn(Permissions.builder().build());
     mockOrganizationsWithoutPermissions(appConfigurationCaptor, "org1", "org2");
 
     ConfigCheckResult checkResult = configValidator.checkConfig();
 
-    assertThat(checkResult.application().autoProvisioning()).isEqualTo(ConfigStatus.failed(MISSING_EMAILS_AND_MEMBERS_PERMISSION));
+    assertThat(checkResult.application().autoProvisioning()).isEqualTo(ConfigStatus.failed(MISSING_ALL_PERMISSIONS));
     assertThat(checkResult.application().jit()).isEqualTo(ConfigStatus.failed(MISSING_EMAIL_PERMISSION));
     assertThat(checkResult.installations()).hasSize(2);
     assertThat(checkResult.installations())
       .extracting(InstallationStatus::jit, InstallationStatus::autoProvisioning)
       .containsExactly(
-        tuple(ConfigStatus.failed(MISSING_EMAIL_PERMISSION), ConfigStatus.failed(MISSING_MEMBERS_PERMISSION)),
-        tuple(ConfigStatus.failed(MISSING_EMAIL_PERMISSION), ConfigStatus.failed(MISSING_MEMBERS_PERMISSION)));
+        tuple(ConfigStatus.failed(MISSING_EMAIL_PERMISSION), ConfigStatus.failed(MISSING_ALL_AUTOPROVISIONNING_PERMISSIONS)),
+        tuple(ConfigStatus.failed(MISSING_EMAIL_PERMISSION), ConfigStatus.failed(MISSING_ALL_AUTOPROVISIONNING_PERMISSIONS)));
     verifyAppConfiguration(appConfigurationCaptor.getValue());
 
   }
 
   @Test
-  public void checkConfig_whenAppDoesntHaveMembersPermissions_shouldReturnFailedAppAutoProvisioningCheck() {
+  public void checkConfig_whenAppDoesntHaveOrgMembersPermissions_shouldReturnFailedAppAutoProvisioningCheck() {
     mockGithubConfiguration();
     ArgumentCaptor<GithubAppConfiguration> appConfigurationCaptor = ArgumentCaptor.forClass(GithubAppConfiguration.class);
 
     GsonApp githubApp = mockGithubApp(appConfigurationCaptor);
-    when(githubApp.getPermissions()).thenReturn(new Permissions(null, null, "read", null));
+    when(githubApp.getPermissions()).thenReturn(Permissions.builder().setEmails("read").build());
     mockOrganizations(appConfigurationCaptor, "org1", "org2");
 
     ConfigCheckResult checkResult = configValidator.checkConfig();
 
     assertThat(checkResult.application().jit()).isEqualTo(ConfigStatus.SUCCESS);
-    assertThat(checkResult.application().autoProvisioning()).isEqualTo(ConfigStatus.failed(MISSING_MEMBERS_PERMISSION));
+    assertThat(checkResult.application().autoProvisioning()).isEqualTo(ConfigStatus.failed(MISSING_ALL_AUTOPROVISIONNING_PERMISSIONS));
     assertThat(checkResult.installations()).hasSize(2);
     verifyAppConfiguration(appConfigurationCaptor.getValue());
   }
@@ -195,7 +197,7 @@ public class GithubProvisioningConfigValidatorTest {
   }
 
   @Test
-  public void checkConfig_whenInstallationsDoesntHaveMembersPermissions_shouldReturnFailedAppAutoProvisioningCheck() {
+  public void checkConfig_whenInstallationsDoesntHaveOrgMembersPermissions_shouldReturnFailedAppAutoProvisioningCheck() {
     mockGithubConfiguration();
     ArgumentCaptor<GithubAppConfiguration> appConfigurationCaptor = ArgumentCaptor.forClass(GithubAppConfiguration.class);
     mockGithubAppWithValidConfig(appConfigurationCaptor);
@@ -206,7 +208,7 @@ public class GithubProvisioningConfigValidatorTest {
     assertSuccessfulAppConfig(checkResult);
     assertThat(checkResult.installations())
       .extracting(InstallationStatus::organization, InstallationStatus::autoProvisioning)
-      .containsExactly(tuple("org1", ConfigStatus.failed(MISSING_MEMBERS_PERMISSION)));
+      .containsExactly(tuple("org1", ConfigStatus.failed(MISSING_ALL_AUTOPROVISIONNING_PERMISSIONS)));
     verifyAppConfiguration(appConfigurationCaptor.getValue());
 
   }
@@ -242,7 +244,7 @@ public class GithubProvisioningConfigValidatorTest {
       .extracting(InstallationStatus::organization, InstallationStatus::autoProvisioning)
       .containsExactlyInAnyOrder(
         tuple("org1", SUCCESS_CHECK),
-        tuple("org2", ConfigStatus.failed(MISSING_MEMBERS_PERMISSION)));
+        tuple("org2", ConfigStatus.failed(MISSING_ALL_AUTOPROVISIONNING_PERMISSIONS)));
     verifyAppConfiguration(appConfigurationCaptor.getValue());
 
   }
@@ -268,7 +270,7 @@ public class GithubProvisioningConfigValidatorTest {
   private GsonApp mockGithubAppWithValidConfig(ArgumentCaptor<GithubAppConfiguration> appConfigurationCaptor) {
     GsonApp githubApp = mock(GsonApp.class);
     when(githubClient.getApp(appConfigurationCaptor.capture())).thenReturn(githubApp);
-    when(githubApp.getPermissions()).thenReturn(new Permissions(null, "read", "read", null));
+    when(githubApp.getPermissions()).thenReturn(Permissions.builder().setMembers("read").setEmails("read").setMetadata("read").setAdministration("read").build());
 
     return githubApp;
   }
@@ -297,15 +299,15 @@ public class GithubProvisioningConfigValidatorTest {
     return installation;
   }
 
-  private static GithubAppInstallation mockInstallationWithMembersPermission(String org) {
+  private static GithubAppInstallation mockInstallationWithAllPermissions(String org) {
     GithubAppInstallation installation = mockInstallation(org);
-    when(installation.permissions()).thenReturn(new Permissions(null, "read", "read", null));
+    when(installation.permissions()).thenReturn(Permissions.builder().setMembers("read").setEmails("read").setMetadata("read").setAdministration("read").build());
     return installation;
   }
 
   private void mockOrganizations(ArgumentCaptor<GithubAppConfiguration> appConfigurationCaptor, String orgWithMembersPermission, String orgWithoutMembersPermission) {
     List<GithubAppInstallation> installations = List.of(
-      mockInstallationWithMembersPermission(orgWithMembersPermission),
+      mockInstallationWithAllPermissions(orgWithMembersPermission),
       mockInstallation(orgWithoutMembersPermission));
     when(githubClient.getWhitelistedGithubAppInstallations(appConfigurationCaptor.capture())).thenReturn(installations);
   }