diff --git a/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleBasedAuthorizationStrategy.java b/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleBasedAuthorizationStrategy.java index ff7f5e7a..8dd70944 100644 --- a/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleBasedAuthorizationStrategy.java +++ b/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleBasedAuthorizationStrategy.java @@ -560,7 +560,7 @@ public void doRemoveTemplates(@QueryParameter(required = true) String names, /** * API method to add a role. * - *

Unknown and dangerous permissions are ignored. + *

Unknown permissions are ignored. * * When specifying a template for an item role, the given permissions are ignored. The named template must exist, * otherwise the request fails with status 400. @@ -1683,9 +1683,6 @@ public List getGroups(@NonNull String type) { public boolean showPermission(String type, Permission p) { switch (type) { case GLOBAL: - if (PermissionHelper.isDangerous(p)) { - return false; - } return p.getEnabled(); case PROJECT: if (!p.isContainedBy(PermissionScope.ITEM_GROUP)) { diff --git a/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleMap.java b/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleMap.java index 250f27fe..dd1831c4 100644 --- a/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleMap.java +++ b/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleMap.java @@ -250,21 +250,14 @@ private static Set getImplyingPermissions(Permission p) { */ private static Set cacheImplyingPermissions(Permission permission) { Set implyingPermissions; - if (PermissionHelper.isDangerous(permission)) { - /* - * if this is a dangerous permission, fall back to Administer unless we're in compat mode - */ - implyingPermissions = getImplyingPermissions(Jenkins.ADMINISTER); - } else { - implyingPermissions = new HashSet<>(); - - // Get the implying permissions - for (Permission p = permission; p != null; p = p.impliedBy) { - if (!p.getEnabled()) { - continue; - } - implyingPermissions.add(p); + implyingPermissions = new HashSet<>(); + + // Get the implying permissions + for (Permission p = permission; p != null; p = p.impliedBy) { + if (!p.getEnabled()) { + continue; } + implyingPermissions.add(p); } return implyingPermissions; diff --git a/src/main/java/org/jenkinsci/plugins/rolestrategy/permissions/PermissionHelper.java b/src/main/java/org/jenkinsci/plugins/rolestrategy/permissions/PermissionHelper.java index 55ba7a80..037c7d0a 100644 --- a/src/main/java/org/jenkinsci/plugins/rolestrategy/permissions/PermissionHelper.java +++ b/src/main/java/org/jenkinsci/plugins/rolestrategy/permissions/PermissionHelper.java @@ -26,10 +26,8 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; -import hudson.PluginManager; import hudson.security.Permission; import hudson.security.PermissionGroup; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -39,12 +37,11 @@ import java.util.logging.Logger; import java.util.regex.Matcher; import java.util.regex.Pattern; -import jenkins.model.Jenkins; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; /** - * Helper methods for dangerous permission handling. + * Helper methods for permission handling. * * @author Oleg Nenashev */ @@ -55,21 +52,13 @@ public class PermissionHelper { private static final Pattern PERMISSION_PATTERN = Pattern.compile("^([^\\/]+)\\/(.+)$"); - /** - * List of the dangerous permissions, which need to be suppressed by the plugin. - */ - @SuppressWarnings("deprecation") - @Restricted(NoExternalUse.class) - public static final Set DANGEROUS_PERMISSIONS = Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(Jenkins.RUN_SCRIPTS, PluginManager.CONFIGURE_UPDATECENTER, PluginManager.UPLOAD_PLUGINS))); - private PermissionHelper() { // Cannot be constructed } /** * Convert a set of string to a collection of permissions. - * Dangerous and non-solvable permissions are ignored + * Non-solvable permissions are ignored * * @param permissionStrings A list of Permission IDs or UI names. * @param allowPermissionId Allow to resolve the permission from the ID. @@ -94,16 +83,6 @@ public static Set fromStrings(@CheckForNull Collection permi return res; } - /** - * Check if the permissions is dangerous. - * - * @param p Permission - * @return {@code true} if the permission is considered as dangerous. - */ - public static boolean isDangerous(Permission p) { - return DANGEROUS_PERMISSIONS.contains(p); - } - /** * Attempt to match a given permission to what is defined in the UI. * @@ -113,7 +92,7 @@ public static boolean isDangerous(Permission p) { @CheckForNull public static Permission findPermission(String id) { final String resolvedId = findPermissionId(id); - return resolvedId != null ? getSafePermission(resolvedId) : null; + return resolvedId != null ? Permission.fromId(resolvedId) : null; } /** @@ -145,23 +124,18 @@ public static String findPermissionId(String id) { } private static @CheckForNull Permission getSafePermission(String id) { - Permission permission = Permission.fromId(id); - if (permission != null && isDangerous(permission)) { - LOGGER.log(Level.WARNING, "The permission: '" + permission + "' is dangerous and will be ignored."); - return null; - } - return permission; + return Permission.fromId(id); } /** * Attempt to match a given permission to what is defined in the UI or from the ID representation used in the config.xml. * * @param id String of the form "Title/Permission" (Look in the UI) for a particular permission or in the form used in the config.xml - * @return a matched permission, null if permission couldn't be resolved or is dangerous + * @return a matched permission, null if permission couldn't be resolved */ @CheckForNull public static Permission resolvePermissionFromString(String id) { - Permission permission = getSafePermission(id); + Permission permission = Permission.fromId(id); if (permission == null) { permission = PermissionHelper.findPermission(id); } diff --git a/src/test/java/com/michelin/cio/hudson/plugins/rolestrategy/ApiTest.java b/src/test/java/com/michelin/cio/hudson/plugins/rolestrategy/ApiTest.java index fda8acc2..011354b7 100644 --- a/src/test/java/com/michelin/cio/hudson/plugins/rolestrategy/ApiTest.java +++ b/src/test/java/com/michelin/cio/hudson/plugins/rolestrategy/ApiTest.java @@ -533,28 +533,6 @@ void testUnassignGroupRole() throws IOException { assertFalse(folder.hasPermission2(alice, Item.CONFIGURE)); } - @Test - void ignoreDangerousPermissionInAddRole() throws IOException { - String roleName = "new-role"; - // Adding role via web request - URL apiUrl = new URL(jenkinsRule.jenkins.getRootUrl() + "role-strategy/strategy/addRole"); - WebRequest request = new WebRequest(apiUrl, HttpMethod.POST); - request.setRequestParameters( - Arrays.asList(new NameValuePair("type", RoleType.Global.getStringType()), new NameValuePair("roleName", roleName), - new NameValuePair("permissionIds", - "hudson.model.Hudson.RunScripts,hudson.model.Hudson.ConfigureUpdateCenter," - + "hudson.model.Hudson.UploadPlugins,hudson.model.Item.Read"), - new NameValuePair("overwrite", "false"))); - Page page = webClient.getPage(request); - assertEquals(HttpURLConnection.HTTP_OK, page.getWebResponse().getStatusCode(), "Testing if request is successful"); - - // Verifying that the role is in - assertThat(rbas.getRoleMap(RoleType.Global).getRole(roleName).hasPermission(PluginManager.CONFIGURE_UPDATECENTER), is(false)); - assertThat(rbas.getRoleMap(RoleType.Global).getRole(roleName).hasPermission(PluginManager.UPLOAD_PLUGINS), is(false)); - assertThat(rbas.getRoleMap(RoleType.Global).getRole(roleName).hasPermission(Jenkins.RUN_SCRIPTS), is(false)); - assertThat(rbas.getRoleMap(RoleType.Global).getRole(roleName).hasPermission(Item.READ), is(true)); - } - @Test void testRemoveRolesAs() throws Exception { diff --git a/src/test/java/org/jenkinsci/plugins/rolestrategy/ConfigurationAsCodeTest.java b/src/test/java/org/jenkinsci/plugins/rolestrategy/ConfigurationAsCodeTest.java index 6f1cdd4e..69da4dd1 100644 --- a/src/test/java/org/jenkinsci/plugins/rolestrategy/ConfigurationAsCodeTest.java +++ b/src/test/java/org/jenkinsci/plugins/rolestrategy/ConfigurationAsCodeTest.java @@ -139,18 +139,6 @@ void shouldHandleNullItemsAndAgentsCorrectly(JenkinsConfiguredWithCodeRule jcwcR assertThat(globalRoles.size(), equalTo(2)); } - @Test - @ConfiguredWithCode("Configuration-as-Code3.yml") - void dangerousPermissionsAreIgnored(JenkinsConfiguredWithCodeRule jcwcRule) { - AuthorizationStrategy s = jcwcRule.jenkins.getAuthorizationStrategy(); - assertThat("Authorization Strategy has been read incorrectly", s, instanceOf(RoleBasedAuthorizationStrategy.class)); - RoleBasedAuthorizationStrategy rbas = (RoleBasedAuthorizationStrategy) s; - - assertThat(rbas.getRoleMap(RoleType.Global).getRole("dangerous").hasPermission(PluginManager.CONFIGURE_UPDATECENTER), is(false)); - assertThat(rbas.getRoleMap(RoleType.Global).getRole("dangerous").hasPermission(PluginManager.UPLOAD_PLUGINS), is(false)); - assertThat(rbas.getRoleMap(RoleType.Global).getRole("dangerous").hasPermission(Jenkins.RUN_SCRIPTS), is(false)); - } - @Test @ConfiguredWithCode("Configuration-as-Code-no-permissions.yml") void exportWithEmptyRole(JenkinsConfiguredWithCodeRule jcwcRule) throws Exception { diff --git a/src/test/java/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest.java b/src/test/java/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest.java index 7e2af8d9..e5b4bf05 100644 --- a/src/test/java/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest.java +++ b/src/test/java/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest.java @@ -1,19 +1,13 @@ package org.jenkinsci.plugins.rolestrategy; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; -import com.michelin.cio.hudson.plugins.rolestrategy.RoleBasedAuthorizationStrategy; import com.michelin.cio.hudson.plugins.rolestrategy.RoleMap; -import com.synopsys.arc.jenkins.plugins.rolestrategy.RoleType; -import hudson.PluginManager; import hudson.model.User; import hudson.security.ACL; import hudson.security.ACLContext; import hudson.security.Permission; -import jenkins.model.Jenkins; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.jvnet.hudson.test.JenkinsRule; @@ -67,13 +61,4 @@ void testRoleAssignmentCaseSensitiveNoMatchFails() { } } - @LocalData - @Test - void dangerousPermissionsAreIgnored() { - RoleBasedAuthorizationStrategy rbas = (RoleBasedAuthorizationStrategy) jenkinsRule.jenkins.getAuthorizationStrategy(); - assertThat(rbas.getRoleMap(RoleType.Global).getRole("POWERUSERS").hasPermission(PluginManager.CONFIGURE_UPDATECENTER), is(false)); - assertThat(rbas.getRoleMap(RoleType.Global).getRole("POWERUSERS").hasPermission(PluginManager.UPLOAD_PLUGINS), is(false)); - assertThat(rbas.getRoleMap(RoleType.Global).getRole("POWERUSERS").hasPermission(Jenkins.RUN_SCRIPTS), is(false)); - } - } diff --git a/src/test/resources/org/jenkinsci/plugins/rolestrategy/Configuration-as-Code3.yml b/src/test/resources/org/jenkinsci/plugins/rolestrategy/Configuration-as-Code3.yml index 07085674..18102aa4 100644 --- a/src/test/resources/org/jenkinsci/plugins/rolestrategy/Configuration-as-Code3.yml +++ b/src/test/resources/org/jenkinsci/plugins/rolestrategy/Configuration-as-Code3.yml @@ -12,9 +12,6 @@ jenkins: - name: "dangerous" description: "Dangerous" permissions: - - "Overall/RunScripts" - - "Overall/ConfigureUpdateCenter" - - "Overall/UploadPlugins" - "Job/Read" entries: - user: "test" diff --git a/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldGrantDangerousPermissionsWhenEnabled/config.xml b/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldGrantDangerousPermissionsWhenEnabled/config.xml deleted file mode 100644 index 7df78006..00000000 --- a/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldGrantDangerousPermissionsWhenEnabled/config.xml +++ /dev/null @@ -1,57 +0,0 @@ - - - - 1.480 - 2 - NORMAL - true - - - - - hudson.model.Hudson.ConfigureUpdateCenter - hudson.model.Hudson.RunScripts - - - fakeAdmin - - - - - hudson.model.Hudson.Read - hudson.model.Job.Read - - - anonymous - - - - - - ${ITEM_ROOTDIR}/workspace - ${ITEM_ROOTDIR}/builds - - false - - - - - - - 5 - 0 - - - - All - false - false - - - - All - 0 - - - - \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldNotShowDangerousPermissionsWhenDisabled/config.xml b/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldNotShowDangerousPermissionsWhenDisabled/config.xml deleted file mode 100644 index 7df78006..00000000 --- a/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldNotShowDangerousPermissionsWhenDisabled/config.xml +++ /dev/null @@ -1,57 +0,0 @@ - - - - 1.480 - 2 - NORMAL - true - - - - - hudson.model.Hudson.ConfigureUpdateCenter - hudson.model.Hudson.RunScripts - - - fakeAdmin - - - - - hudson.model.Hudson.Read - hudson.model.Job.Read - - - anonymous - - - - - - ${ITEM_ROOTDIR}/workspace - ${ITEM_ROOTDIR}/builds - - false - - - - - - - 5 - 0 - - - - All - false - false - - - - All - 0 - - - - \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldNotShowDangerousWhenAdmin/config.xml b/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldNotShowDangerousWhenAdmin/config.xml deleted file mode 100644 index 1f65bd52..00000000 --- a/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldNotShowDangerousWhenAdmin/config.xml +++ /dev/null @@ -1,57 +0,0 @@ - - - - 1.480 - 2 - NORMAL - true - - - - - hudson.model.Hudson.Administer - hudson.model.Hudson.RunScripts - - - admin - - - - - hudson.model.Hudson.Read - hudson.model.Job.Read - - - anonymous - - - - - - ${ITEM_ROOTDIR}/workspace - ${ITEM_ROOTDIR}/builds - - false - - - - - - - 5 - 0 - - - - All - false - false - - - - All - 0 - - - - \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldNotShowDangerousWhenNotAssigned/config.xml b/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldNotShowDangerousWhenNotAssigned/config.xml deleted file mode 100644 index a94763b3..00000000 --- a/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldNotShowDangerousWhenNotAssigned/config.xml +++ /dev/null @@ -1,56 +0,0 @@ - - - - 1.480 - 2 - NORMAL - true - - - - - hudson.model.Hudson.Administer - - - admin - - - - - hudson.model.Hudson.Read - hudson.model.Job.Read - - - anonymous - - - - - - ${ITEM_ROOTDIR}/workspace - ${ITEM_ROOTDIR}/builds - - false - - - - - - - 5 - 0 - - - - All - false - false - - - - All - 0 - - - - \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldShowDangerousWhenAssigned/config.xml b/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldShowDangerousWhenAssigned/config.xml deleted file mode 100644 index 7df78006..00000000 --- a/src/test/resources/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest/shouldShowDangerousWhenAssigned/config.xml +++ /dev/null @@ -1,57 +0,0 @@ - - - - 1.480 - 2 - NORMAL - true - - - - - hudson.model.Hudson.ConfigureUpdateCenter - hudson.model.Hudson.RunScripts - - - fakeAdmin - - - - - hudson.model.Hudson.Read - hudson.model.Job.Read - - - anonymous - - - - - - ${ITEM_ROOTDIR}/workspace - ${ITEM_ROOTDIR}/builds - - false - - - - - - - 5 - 0 - - - - All - false - false - - - - All - 0 - - - - \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest/dangerousPermissionsAreIgnored/config.xml b/src/test/resources/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest/dangerousPermissionsAreIgnored/config.xml deleted file mode 100644 index 8c458761..00000000 --- a/src/test/resources/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest/dangerousPermissionsAreIgnored/config.xml +++ /dev/null @@ -1,58 +0,0 @@ - - - - 2.303.3 - 2 - NORMAL - true - - - - - hudson.model.Hudson.Administer - - - alice - - - - - hudson.model.Hudson.RunScripts - hudson.model.Hudson.ConfigureUpdateCenter - hudson.model.Hudson.UploadPlugins - - - berta - - - - - - - ${ITEM_ROOTDIR}/workspace - ${ITEM_ROOTDIR}/builds - - false - - - - - - - 5 - 0 - - - - All - false - false - - - - All - 0 - - - - \ No newline at end of file