From 55b2bc7f3fe037e46a06deb44b93b02787df28ef Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Fri, 14 Feb 2020 19:33:19 +0100 Subject: [PATCH] [JEP-223] Overall/Manage permission Co-Authored-By: Esther Alvarez Feijoo Co-Authored-By: aHenryJard Co-Authored-By: michael cirioli --- core/src/main/java/hudson/FilePath.java | 2 +- core/src/main/java/hudson/Functions.java | 4 ++++ .../main/java/hudson/model/Descriptor.java | 14 +++++++++++ .../hudson/model/ManageJenkinsAction.java | 2 +- .../java/hudson/model/ManagementLink.java | 2 +- .../main/java/jenkins/management/CliLink.java | 10 ++++++++ .../jenkins/management/ConfigureLink.java | 10 ++++++++ .../java/jenkins/management/NodesLink.java | 10 ++++++++ .../jenkins/management/StatisticsLink.java | 10 ++++++++ ...balProjectNamingStrategyConfiguration.java | 9 ++++++++ .../model/GlobalQuietPeriodConfiguration.java | 8 +++++++ .../GlobalSCMRetryCountConfiguration.java | 8 +++++++ core/src/main/java/jenkins/model/Jenkins.java | 23 ++++++++++++++++++- .../hudson/PluginManager/sidepanel.groovy | 2 +- .../hudson/model/ComputerSet/sidepanel.jelly | 2 +- .../hudson/model/Messages.properties | 4 ++++ .../sidepanel.jelly | 2 +- .../jenkins/model/Jenkins/configure.jelly | 2 +- .../jenkins/model/Jenkins/manage.jelly | 2 +- 19 files changed, 117 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index 01a8d3cc0e..6b5cf97304 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -2971,7 +2971,7 @@ public final class FilePath implements SerializableOnlyOverRemoting { private static void checkPermissionForValidate() { AccessControlled subject = Stapler.getCurrentRequest().findAncestorObject(AbstractProject.class); if (subject == null) - Jenkins.get().checkPermission(Jenkins.ADMINISTER); + Jenkins.get().checkPermission(Jenkins.MANAGE); else subject.checkPermission(Item.CONFIGURE); } diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java index 50339569b6..62208aaf93 100644 --- a/core/src/main/java/hudson/Functions.java +++ b/core/src/main/java/hudson/Functions.java @@ -1062,6 +1062,10 @@ public class Functions { Descriptor d = c.getInstance(); if (d.getGlobalConfigPage()==null) continue; + if (!Jenkins.get().hasPermission(d.getPermission())) { + continue; + } + if (predicate.apply(d.getCategory())) { r.add(new Tag(c.ordinal(), d)); } diff --git a/core/src/main/java/hudson/model/Descriptor.java b/core/src/main/java/hudson/model/Descriptor.java index 463cdee366..3a055d8119 100644 --- a/core/src/main/java/hudson/model/Descriptor.java +++ b/core/src/main/java/hudson/model/Descriptor.java @@ -24,6 +24,7 @@ package hudson.model; import hudson.DescriptorExtensionList; +import hudson.Extension; import hudson.PluginWrapper; import hudson.RelativePath; import hudson.XmlFile; @@ -31,6 +32,7 @@ import hudson.BulkChange; import hudson.ExtensionList; import hudson.Util; import hudson.model.listeners.SaveableListener; +import hudson.security.Permission; import hudson.util.FormApply; import hudson.util.FormValidation.CheckMethod; import hudson.util.ReflectionUtils; @@ -830,6 +832,18 @@ public abstract class Descriptor> implements Saveable, return GlobalConfigurationCategory.get(GlobalConfigurationCategory.Unclassified.class); } + /** + * Returns the permission type needed in order to configure the descriptor if and only if it is configured through the global (Unclassified) configuration. + * By default, requires {@link Jenkins.ADMINISTER} permission. + * Override to return something different if appropriate. The only currently supported alternative return value is {@link Jenkins.MANAGE}. + * + * @return Permission required to configure this descriptor. + */ + public @Nonnull + Permission getPermission() { + return Jenkins.ADMINISTER; + } + private String getViewPage(Class clazz, String pageName, String defaultValue) { return getViewPage(clazz,Collections.singleton(pageName),defaultValue); } diff --git a/core/src/main/java/hudson/model/ManageJenkinsAction.java b/core/src/main/java/hudson/model/ManageJenkinsAction.java index b1fb854a10..06659b19d8 100644 --- a/core/src/main/java/hudson/model/ManageJenkinsAction.java +++ b/core/src/main/java/hudson/model/ManageJenkinsAction.java @@ -35,7 +35,7 @@ import org.jenkinsci.Symbol; @Extension(ordinal=100) @Symbol("manageJenkins") public class ManageJenkinsAction implements RootAction { public String getIconFileName() { - if (Jenkins.get().hasPermission(Jenkins.ADMINISTER)) + if (Jenkins.get().hasPermission(Jenkins.MANAGE)) return "gear2.png"; else return null; diff --git a/core/src/main/java/hudson/model/ManagementLink.java b/core/src/main/java/hudson/model/ManagementLink.java index 00d6871097..05db6030f0 100644 --- a/core/src/main/java/hudson/model/ManagementLink.java +++ b/core/src/main/java/hudson/model/ManagementLink.java @@ -115,7 +115,7 @@ public abstract class ManagementLink implements ExtensionPoint, Action { * @return permission required for user to access this management link, in addition to {@link Jenkins#ADMINISTER} */ public @CheckForNull Permission getRequiredPermission() { - return null; + return Jenkins.ADMINISTER; } /** diff --git a/core/src/main/java/jenkins/management/CliLink.java b/core/src/main/java/jenkins/management/CliLink.java index 718504727f..6a2c67f791 100644 --- a/core/src/main/java/jenkins/management/CliLink.java +++ b/core/src/main/java/jenkins/management/CliLink.java @@ -26,8 +26,12 @@ package jenkins.management; import hudson.Extension; import hudson.model.ManagementLink; +import hudson.security.Permission; +import jenkins.model.Jenkins; import org.jenkinsci.Symbol; +import javax.annotation.CheckForNull; + /** * @author Nicolas De Loof */ @@ -48,6 +52,12 @@ public class CliLink extends ManagementLink { return Messages.CliLink_Description(); } + @CheckForNull + @Override + public Permission getRequiredPermission() { + return Jenkins.MANAGE; + } + @Override public String getUrlName() { return "cli"; diff --git a/core/src/main/java/jenkins/management/ConfigureLink.java b/core/src/main/java/jenkins/management/ConfigureLink.java index a10df71b22..61fe8f1103 100644 --- a/core/src/main/java/jenkins/management/ConfigureLink.java +++ b/core/src/main/java/jenkins/management/ConfigureLink.java @@ -26,8 +26,12 @@ package jenkins.management; import hudson.Extension; import hudson.model.ManagementLink; +import hudson.security.Permission; +import jenkins.model.Jenkins; import org.jenkinsci.Symbol; +import javax.annotation.CheckForNull; + /** * @author Nicolas De Loof */ @@ -48,6 +52,12 @@ public class ConfigureLink extends ManagementLink { return Messages.ConfigureLink_Description(); } + @CheckForNull + @Override + public Permission getRequiredPermission() { + return Jenkins.MANAGE; + } + @Override public String getUrlName() { return "configure"; diff --git a/core/src/main/java/jenkins/management/NodesLink.java b/core/src/main/java/jenkins/management/NodesLink.java index 90038450ce..3f85b701b9 100644 --- a/core/src/main/java/jenkins/management/NodesLink.java +++ b/core/src/main/java/jenkins/management/NodesLink.java @@ -26,9 +26,13 @@ package jenkins.management; import hudson.Extension; import hudson.model.ManagementLink; +import hudson.security.Permission; import jenkins.management.Messages; +import jenkins.model.Jenkins; import org.jenkinsci.Symbol; +import javax.annotation.CheckForNull; + /** * @author Nicolas De Loof */ @@ -49,6 +53,12 @@ public class NodesLink extends ManagementLink { return Messages.NodesLink_Description(); } + @CheckForNull + @Override + public Permission getRequiredPermission() { + return Jenkins.MANAGE; + } + @Override public String getUrlName() { return "computer"; diff --git a/core/src/main/java/jenkins/management/StatisticsLink.java b/core/src/main/java/jenkins/management/StatisticsLink.java index 9821781d78..83ff09edd3 100644 --- a/core/src/main/java/jenkins/management/StatisticsLink.java +++ b/core/src/main/java/jenkins/management/StatisticsLink.java @@ -26,8 +26,12 @@ package jenkins.management; import hudson.Extension; import hudson.model.ManagementLink; +import hudson.security.Permission; +import jenkins.model.Jenkins; import org.jenkinsci.Symbol; +import javax.annotation.CheckForNull; + /** * @author Nicolas De Loof */ @@ -48,6 +52,12 @@ public class StatisticsLink extends ManagementLink { return Messages.StatisticsLink_Description(); } + @CheckForNull + @Override + public Permission getRequiredPermission() { + return Jenkins.MANAGE; + } + @Override public String getUrlName() { return "load-statistics"; diff --git a/core/src/main/java/jenkins/model/GlobalProjectNamingStrategyConfiguration.java b/core/src/main/java/jenkins/model/GlobalProjectNamingStrategyConfiguration.java index 2502df1b6e..4f046291f2 100644 --- a/core/src/main/java/jenkins/model/GlobalProjectNamingStrategyConfiguration.java +++ b/core/src/main/java/jenkins/model/GlobalProjectNamingStrategyConfiguration.java @@ -24,12 +24,15 @@ package jenkins.model; import hudson.Extension; +import hudson.security.Permission; import jenkins.model.ProjectNamingStrategy.DefaultProjectNamingStrategy; import net.sf.json.JSONObject; import org.jenkinsci.Symbol; import org.kohsuke.stapler.StaplerRequest; +import javax.annotation.Nonnull; + /** * Configures the project naming strategy. * @@ -59,4 +62,10 @@ public class GlobalProjectNamingStrategyConfiguration extends GlobalConfiguratio } return true; } + + @Nonnull + @Override + public Permission getPermission() { + return Jenkins.MANAGE; + } } diff --git a/core/src/main/java/jenkins/model/GlobalQuietPeriodConfiguration.java b/core/src/main/java/jenkins/model/GlobalQuietPeriodConfiguration.java index b0cdd84d3d..93495cd384 100644 --- a/core/src/main/java/jenkins/model/GlobalQuietPeriodConfiguration.java +++ b/core/src/main/java/jenkins/model/GlobalQuietPeriodConfiguration.java @@ -24,10 +24,12 @@ package jenkins.model; import hudson.Extension; +import hudson.security.Permission; import net.sf.json.JSONObject; import org.jenkinsci.Symbol; import org.kohsuke.stapler.StaplerRequest; +import javax.annotation.Nonnull; import java.io.IOException; /** @@ -57,4 +59,10 @@ public class GlobalQuietPeriodConfiguration extends GlobalConfiguration { throw new FormException(e,"quietPeriod"); } } + + @Nonnull + @Override + public Permission getPermission() { + return Jenkins.MANAGE; + } } diff --git a/core/src/main/java/jenkins/model/GlobalSCMRetryCountConfiguration.java b/core/src/main/java/jenkins/model/GlobalSCMRetryCountConfiguration.java index 32d0c155c6..373eb62828 100644 --- a/core/src/main/java/jenkins/model/GlobalSCMRetryCountConfiguration.java +++ b/core/src/main/java/jenkins/model/GlobalSCMRetryCountConfiguration.java @@ -24,11 +24,13 @@ package jenkins.model; import hudson.Extension; +import hudson.security.Permission; import net.sf.json.JSONException; import net.sf.json.JSONObject; import org.jenkinsci.Symbol; import org.kohsuke.stapler.StaplerRequest; +import javax.annotation.Nonnull; import java.io.IOException; /** @@ -54,4 +56,10 @@ public class GlobalSCMRetryCountConfiguration extends GlobalConfiguration { throw new FormException(e.getMessage(), "quietPeriod"); } } + + @Nonnull + @Override + public Permission getPermission() { + return Jenkins.MANAGE; + } } diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 065251d881..9a44b86e6f 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -2199,6 +2199,9 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve * @since 2.64 */ public List getActiveAdministrativeMonitors() { + if (!Jenkins.get().hasPermission(ADMINISTER)) { + return Collections.emptyList(); + } return administrativeMonitors.stream().filter(m -> { try { return m.isEnabled() && m.isActivated(); @@ -3762,7 +3765,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve public synchronized void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, FormException { BulkChange bc = new BulkChange(this); try { - checkPermission(ADMINISTER); + checkPermission(MANAGE); JSONObject json = req.getSubmittedForm(); @@ -5236,7 +5239,25 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve private static final Logger LOGGER = Logger.getLogger(Jenkins.class.getName()); public static final PermissionGroup PERMISSIONS = Permission.HUDSON_PERMISSIONS; + /** + * Grants ability to configure any and all aspects of the Jenkins instance + */ public static final Permission ADMINISTER = Permission.HUDSON_ADMINISTER; + + /** + * Allows non-privilege escalating configuration permission for a Jenkins instance. Actions which could result + * in a privilege escalation (such as RUN_SCRIPTS) require explicit ADMINISTER permission. + * + * As an experimental feature, making the manage permission able to be disabled by default (keep as ADMINISTER), can + * be enabled with "jenkins.security.ManagePermission" system property. + */ + public static final Permission MANAGE = new Permission(PERMISSIONS, "Manage", + Messages._Jenkins_Manage_Description(), + ADMINISTER, + SystemProperties.getBoolean("jenkins.security.ManagePermission"), + new PermissionScope[]{PermissionScope.JENKINS}); + + public static final Permission READ = new Permission(PERMISSIONS,"Read",Messages._Hudson_ReadPermission_Description(),Permission.READ,PermissionScope.JENKINS); public static final Permission RUN_SCRIPTS = new Permission(PERMISSIONS, "RunScripts", Messages._Hudson_RunScriptsPermission_Description(),ADMINISTER,PermissionScope.JENKINS); diff --git a/core/src/main/resources/hudson/PluginManager/sidepanel.groovy b/core/src/main/resources/hudson/PluginManager/sidepanel.groovy index 375fda352d..a3aa54878d 100644 --- a/core/src/main/resources/hudson/PluginManager/sidepanel.groovy +++ b/core/src/main/resources/hudson/PluginManager/sidepanel.groovy @@ -28,7 +28,7 @@ l.header() l.side_panel { l.tasks { l.task(icon:"icon-up icon-md", href:rootURL+'/', title:_("Back to Dashboard")) - l.task(icon:"icon-gear2 icon-md", href:"${rootURL}/manage", title:_("Manage Jenkins"), permission:app.ADMINISTER, it:app) + l.task(icon:"icon-gear2 icon-md", href:"${rootURL}/manage", title:_("Manage Jenkins"), permission:app.MANAGE, it:app) if (!app.updateCenter.jobs.isEmpty()) { l.task(icon:"icon-plugin icon-md", href:"${rootURL}/updateCenter/", title:_("Update Center")) } diff --git a/core/src/main/resources/hudson/model/ComputerSet/sidepanel.jelly b/core/src/main/resources/hudson/model/ComputerSet/sidepanel.jelly index 1a9a257225..919e67e648 100644 --- a/core/src/main/resources/hudson/model/ComputerSet/sidepanel.jelly +++ b/core/src/main/resources/hudson/model/ComputerSet/sidepanel.jelly @@ -32,7 +32,7 @@ THE SOFTWARE. - + diff --git a/core/src/main/resources/hudson/model/Messages.properties b/core/src/main/resources/hudson/model/Messages.properties index 8807077497..5e1553e12a 100644 --- a/core/src/main/resources/hudson/model/Messages.properties +++ b/core/src/main/resources/hudson/model/Messages.properties @@ -163,6 +163,10 @@ Hudson.AdministerPermission.Description=\ This permission grants the ability to make system-wide configuration changes, \ as well as perform highly sensitive operations that amounts to full local system access \ (within the scope granted by the underlying OS.) +Jenkins.Manage.Description=\ + This permission grants the ability to make non Permission escalating system \ + configuration changes. \ + System configuration which would allow the execution of arbitrary commands or code on the master requires Administer. Hudson.ReadPermission.Description=\ The read permission is necessary for viewing almost all pages of Jenkins. \ This permission is useful when you don\u2019t want unauthenticated users to see \ diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/sidepanel.jelly b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/sidepanel.jelly index 8d864d01e9..6cc9f44569 100644 --- a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/sidepanel.jelly +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/sidepanel.jelly @@ -28,7 +28,7 @@ THE SOFTWARE. - + diff --git a/core/src/main/resources/jenkins/model/Jenkins/configure.jelly b/core/src/main/resources/jenkins/model/Jenkins/configure.jelly index a85ab58ade..6c6cfe895e 100644 --- a/core/src/main/resources/jenkins/model/Jenkins/configure.jelly +++ b/core/src/main/resources/jenkins/model/Jenkins/configure.jelly @@ -27,7 +27,7 @@ THE SOFTWARE. --> - + diff --git a/core/src/main/resources/jenkins/model/Jenkins/manage.jelly b/core/src/main/resources/jenkins/model/Jenkins/manage.jelly index d9dd6de40d..16a54119f6 100644 --- a/core/src/main/resources/jenkins/model/Jenkins/manage.jelly +++ b/core/src/main/resources/jenkins/model/Jenkins/manage.jelly @@ -27,7 +27,7 @@ THE SOFTWARE. --> - +