From: Antoine Vigneau Date: Wed, 5 Jul 2023 14:19:20 +0000 (+0200) Subject: SONAR-19785 Add required new permissions in the GitHub config check X-Git-Tag: 10.2.0.77647~395 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=7ad2ec83dcede27ee5541740c08d3281ce3eed98;p=sonarqube.git 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 779fefbadd5..8296839a3e1 100644 --- 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 efa137121c2..9ed7e9fed67 100644 --- 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 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 missingPermissions) { - return ConfigStatus.failed("Missing permissions: " + String.join(",", missingPermissions)); + return ConfigStatus.failed("Missing GitHub permissions: " + String.join(", ", missingPermissions)); } private List checkInstallations(GithubAppConfiguration githubAppConfiguration, ApplicationStatus appStatus) { @@ -158,10 +166,20 @@ public class GithubProvisioningConfigValidator { } private static ConfigStatus autoProvisioningInstallationConfigStatus(Permissions permissions) { + List 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 2d64e2b4a5c..f3191bc3a92 100644 --- 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 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 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 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 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 appConfigurationCaptor, String orgWithMembersPermission, String orgWithoutMembersPermission) { List installations = List.of( - mockInstallationWithMembersPermission(orgWithMembersPermission), + mockInstallationWithAllPermissions(orgWithMembersPermission), mockInstallation(orgWithoutMembersPermission)); when(githubClient.getWhitelistedGithubAppInstallations(appConfigurationCaptor.capture())).thenReturn(installations); }