From d85919c54ad6163e84ea3e8a44281aa5d35579ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Garc=C3=ADa?= <60674455+jgarciacloudbees@users.noreply.github.com> Date: Tue, 12 Nov 2024 19:32:17 +0100 Subject: [PATCH] JENKINS-74820 - forceSandBox - Hide command-launcher drop down from non-administrators (#103) * BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators * BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators * BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators - Tests * BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators - Tests * BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators - Tests * BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators - Tests * BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators - Tests * BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators - Tests * JENKINS-74820 - Apply suggestions from code review Co-authored-by: Antonio Muniz * BEE-52312 - change constructor signature to avoid breaking changes. * BEE-52312 - bump pom version * JENKINS-74820 - Apply suggestions from code review Co-authored-by: Jesse Glick * BEE-52312 - add pom dependecies * BEE-52312 - add pom dependecies * BEE-52312 - SuggestedChanges * BEE-52312 - SuggestedChanges - Test * BEE-52312 - final changes --------- Co-authored-by: Antonio Muniz Co-authored-by: Jesse Glick --- pom.xml | 8 +- .../java/hudson/slaves/CommandLauncher.java | 64 +++++- .../CommandLauncherForceSandboxTest.java | 202 ++++++++++++++++++ 3 files changed, 269 insertions(+), 5 deletions(-) create mode 100644 src/test/java/hudson/slaves/CommandLauncherForceSandboxTest.java diff --git a/pom.xml b/pom.xml index f9f1f56..2b69c2c 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ org.jenkins-ci.plugins plugin - 4.86 + 4.88 command-launcher @@ -13,7 +13,7 @@ 999999-SNAPSHOT jenkinsci/${project.artifactId}-plugin - 2.440.3 + 2.452.4 Command Agent Launcher Plugin Allows agents to be launched using a specified command. @@ -46,8 +46,8 @@ io.jenkins.tools.bom - bom-2.440.x - 3234.v5ca_5154341ef + bom-2.452.x + 3654.v237e4a_f2d8da_ pom import diff --git a/src/main/java/hudson/slaves/CommandLauncher.java b/src/main/java/hudson/slaves/CommandLauncher.java index e7f3d7c..d8dc9bb 100644 --- a/src/main/java/hudson/slaves/CommandLauncher.java +++ b/src/main/java/hudson/slaves/CommandLauncher.java @@ -23,6 +23,7 @@ */ package hudson.slaves; +import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; import hudson.AbortException; @@ -30,7 +31,9 @@ import hudson.Extension; import hudson.Functions; import hudson.Util; +import hudson.model.ComputerSet; import hudson.model.Descriptor; +import hudson.model.DescriptorVisibilityFilter; import hudson.model.Slave; import hudson.model.TaskListener; import hudson.remoting.Channel; @@ -39,6 +42,7 @@ import hudson.util.StreamCopyThread; import java.io.IOException; import java.util.Date; +import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; import jenkins.model.Jenkins; @@ -49,6 +53,7 @@ import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; import org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedUsageException; import org.jenkinsci.plugins.scriptsecurity.scripts.languages.SystemCommandLanguage; +import org.kohsuke.stapler.Ancestor; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.Stapler; @@ -80,7 +85,8 @@ public class CommandLauncher extends ComputerLauncher { * @see #CommandLauncher(String command, EnvVars env) */ @DataBoundConstructor - public CommandLauncher(String command) { + public CommandLauncher(String command) throws Descriptor.FormException { + checkSandbox(); agentCommand = command; env = null; // TODO add withKey if we can determine the Slave.nodeName being configured @@ -105,6 +111,18 @@ public CommandLauncher(String command, EnvVars env) { this.agentCommand = command; this.env = env; } + + /** + * Check if the current user is forced to use the Sandbox when creating a new instance. + * In this case, we don't allow saving new instances of the CommandLauncher object by throwing a new exception + */ + private void checkSandbox() throws Descriptor.FormException { + if (ScriptApproval.get().isForceSandboxForCurrentUser()) { + throw new Descriptor.FormException( + "This Launch Method requires scripts executions out of the sandbox." + + " This Jenkins instance has been configured to not allow regular users to disable the sandbox", "command"); + } + } private Object readResolve() { ScriptApproval.get().configuring(agentCommand, SystemCommandLanguage.get(), ApprovalContext.create(), true); @@ -250,4 +268,48 @@ public FormValidation doCheckCommand(@QueryParameter String value, @QueryParamet return ScriptApproval.get().checking(value, SystemCommandLanguage.get(), !StringUtils.equals(value, oldCommand)); } } + + /** + * In case the flag + * {@link ScriptApproval#isForceSandboxForCurrentUser} is true, we don't show the {@link DescriptorImpl descriptor} + * for the current user, except if we are editing a node that already has the launcher {@link CommandLauncher} + */ + @Extension + public static class DescriptorVisibilityFilterForceSandBox extends DescriptorVisibilityFilter { + @Override + public boolean filter(@CheckForNull Object context, @NonNull Descriptor descriptor) { + if(descriptor instanceof DescriptorImpl) { + return !ScriptApproval.get().isForceSandboxForCurrentUser() || + (context instanceof Slave && ((Slave) context).getLauncher() instanceof CommandLauncher); + } + return true; + } + + @Override + public boolean filterType(@NonNull Class contextClass, @NonNull Descriptor descriptor) { + if(descriptor instanceof DescriptorImpl) + { + //If we are creating a new object, check ScriptApproval.get().isForceSandboxForCurrentUser() + //If we are NOT creating a new object, return true, and delegate the logic to #filter + return !(isCreatingNewObject() && ScriptApproval.get().isForceSandboxForCurrentUser()); + } + return true; + } + + private boolean isCreatingNewObject() { + var req = Stapler.getCurrentRequest(); + if (req != null) { + List ancs = req.getAncestors(); + for (Ancestor anc : ancs) { + if (anc.getObject() instanceof ComputerSet) { + String uri = req.getOriginalRequestURI(); + if (uri.endsWith("createItem")) { + return true; + } + } + } + } + return false; + } + } } diff --git a/src/test/java/hudson/slaves/CommandLauncherForceSandboxTest.java b/src/test/java/hudson/slaves/CommandLauncherForceSandboxTest.java new file mode 100644 index 0000000..63f42d1 --- /dev/null +++ b/src/test/java/hudson/slaves/CommandLauncherForceSandboxTest.java @@ -0,0 +1,202 @@ +package hudson.slaves; + +import java.io.IOException; + +import org.htmlunit.html.HtmlForm; +import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.JenkinsRule.WebClient; +import org.jvnet.hudson.test.MockAuthorizationStrategy; + +import hudson.model.Computer; +import hudson.model.Descriptor; +import hudson.model.User; +import hudson.security.ACL; +import hudson.security.ACLContext; +import jenkins.model.Jenkins; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +public class CommandLauncherForceSandboxTest { + + @Rule + public JenkinsRule j = new JenkinsRule(); + + @Before + public void configureTest() throws IOException { + Jenkins.MANAGE.setEnabled(true); + + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + + MockAuthorizationStrategy strategy = new MockAuthorizationStrategy(). + grant(Jenkins.ADMINISTER).everywhere().to("admin"). + grant(Jenkins.MANAGE).everywhere().to("devel"). + grant(Jenkins.READ, Computer.CONFIGURE).everywhere().to("devel"); + + SlaveComputer.PERMISSIONS.getPermissions().forEach(p -> strategy.grant(p).everywhere().to("devel")); + + j.jenkins.setAuthorizationStrategy(strategy); + } + + @Test + public void newCommandLauncher() throws Exception { + try (ACLContext ctx = ACL.as(User.getById("devel", true))) { + //With forceSandbox enabled, nonadmin users should not create agents with Launcher = CommandLauncher + ScriptApproval.get().setForceSandbox(true); + Descriptor.FormException ex = assertThrows(Descriptor.FormException.class, () -> + new DumbSlave("s", "/",new CommandLauncher("echo unconfigured"))); + + assertEquals("This Launch Method requires scripts executions out of the sandbox." + + " This Jenkins instance has been configured to not allow regular users to disable the sandbox", + ex.getMessage()); + + //With forceSandbox disabled, nonadmin users can create agents with Launcher = CommandLauncher + ScriptApproval.get().setForceSandbox(false); + new DumbSlave("s", "/", new CommandLauncher("echo unconfigured")); + } + + try (ACLContext ctx = ACL.as(User.getById("admin", true))) { + //admin users can create agents with Launcher = CommandLauncher independently of forceSandbox flag. + ScriptApproval.get().setForceSandbox(true); + new DumbSlave("s", "/", new CommandLauncher("echo unconfigured")); + + ScriptApproval.get().setForceSandbox(false); + new DumbSlave("s", "/", new CommandLauncher("echo unconfigured")); + } + } + + @Test + public void editCommandLauncherUI_ForceSandboxTrue() throws Exception { + ScriptApproval.get().setForceSandbox(true); + + DumbSlave commandLauncherAgent = new DumbSlave("commandLauncherAgent", "/", new CommandLauncher("echo unconfigured")); + DumbSlave noCommandLauncherAgent = new DumbSlave("noCommandLauncherAgent", "/", new JNLPLauncher()); + j.jenkins.addNode(commandLauncherAgent); + j.jenkins.addNode(noCommandLauncherAgent); + + try (WebClient wc = j.createWebClient().login("devel")) { + //Edit noCommandLauncher Agent. + //We are not admin and Sandbox is true, + //We don't have any html object for CommandLauncher + HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); + assertTrue(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + + //Edit CommandLauncher Agent. + //We are not admin and Sandbox is true + // As the agent is already a commandLauncher one we have some html object for CommandLauncher + form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + + try (WebClient wc = j.createWebClient().login("admin")) { + //Edit noCommandLauncher Agent. + //We areadmin and Sandbox is true, + //We have some html object for CommandLauncher + HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + + //Edit CommandLauncher Agent. + //Wwe not admin and Sandbox is true + //We have some html object for CommandLauncher + form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } } + + @Test + public void editCommandLauncherUI_ForceSandboxFalse() throws Exception { + ScriptApproval.get().setForceSandbox(false); + + DumbSlave commandLauncherAgent = new DumbSlave("commandLauncherAgent", "/", new CommandLauncher("echo unconfigured")); + DumbSlave noCommandLauncherAgent = new DumbSlave("noCommandLauncherAgent", "/", new JNLPLauncher()); + j.jenkins.addNode(commandLauncherAgent); + j.jenkins.addNode(noCommandLauncherAgent); + + try (WebClient wc = j.createWebClient().login("devel")) { + //Edit noCommandLauncher Agent. + //We are not admin and Sandbox is false, + //We have some html object for CommandLauncher + HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + + //Edit CommandLauncher Agent. + //Wwe are not admin and Sandbox is false + //We have some html object for CommandLauncher + form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + + try (WebClient wc = j.createWebClient().login("admin")) { + //Edit noCommandLauncher Agent. + //We areadmin and Sandbox is false, + //We have some html object for CommandLauncher + HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + + //Edit CommandLauncher Agent. + //Wwe not admin and Sandbox is false + //We have some html object for CommandLauncher + form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + } + + @Test + public void createCommandLauncherUI_ForceSandboxTrue() throws Exception { + ScriptApproval.get().setForceSandbox(true); + + try (WebClient wc = j.createWebClient().login("devel")) { + //Create Permanent Agent. + //We are not admin and Sandbox is true, + //We don't have any html object for CommandLauncher + HtmlForm form = wc.goTo("computer/new").getFormByName("createItem"); + form.getInputByName("name").setValue("devel_ComandLauncher"); + form.getInputsByValue(DumbSlave.class.getName()).stream().findFirst().get().setChecked(true); + HtmlForm createNodeForm = j.submit(form).getFormByName("config"); + assertTrue(createNodeForm.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + + try (WebClient wc = j.createWebClient().login("admin")) { + //Create Permanent Agent. + //We are admin and Sandbox is true, + //We have some html object for CommandLauncher + HtmlForm form = wc.goTo("computer/new").getFormByName("createItem"); + form.getInputByName("name").setValue("devel_ComandLauncher"); + form.getInputsByValue(DumbSlave.class.getName()).stream().findFirst().get().setChecked(true); + HtmlForm createNodeForm = j.submit(form).getFormByName("config"); + assertFalse(createNodeForm.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + } + + @Test + public void createCommandLauncherUI_ForceSandboxFalse() throws Exception { + ScriptApproval.get().setForceSandbox(false); + + try (WebClient wc = j.createWebClient().login("devel")) { + //Create Permanent Agent. + //We are not admin and Sandbox is false, + //We have some html object for CommandLauncher + HtmlForm form = wc.goTo("computer/new").getFormByName("createItem"); + form.getInputByName("name").setValue("devel_ComandLauncher"); + form.getInputsByValue(DumbSlave.class.getName()).stream().findFirst().get().setChecked(true); + HtmlForm createNodeForm = j.submit(form).getFormByName("config"); + assertFalse(createNodeForm.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + + try (WebClient wc = j.createWebClient().login("admin")) { + //Create Permanent Agent. + //We are admin and Sandbox is true, + //We have some html object for CommandLauncher + HtmlForm form = wc.goTo("computer/new").getFormByName("createItem"); + form.getInputByName("name").setValue("devel_ComandLauncher"); + form.getInputsByValue(DumbSlave.class.getName()).stream().findFirst().get().setChecked(true); + HtmlForm createNodeForm = j.submit(form).getFormByName("config"); + assertFalse(createNodeForm.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + } +}