diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java index 073d14e4..37148175 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java @@ -96,9 +96,8 @@ public final class SecureGroovyScript extends AbstractDescribableImpl classpath) throws Descriptor.FormException { - if (!sandbox && ScriptApproval.get().isForceSandboxForCurrentUser()) { - throw new Descriptor.FormException(Messages.ScriptApproval_SandboxCantBeDisabled(), "sandbox"); - } + ScriptApproval.validateSandbox(sandbox); + this.script = script; this.sandbox = sandbox; this.classpath = classpath; @@ -473,8 +472,7 @@ public FormValidation doCheckScript(@QueryParameter String value, @QueryParamete public boolean shouldHideSandbox(@CheckForNull SecureGroovyScript instance) { // sandbox checkbox is shown to admins even if the global configuration says otherwise // it's also shown when sandbox == false, so regular users can enable it - return ScriptApproval.get().isForceSandboxForCurrentUser() - && (instance == null || instance.sandbox); + return ScriptApproval.shouldHideSandbox(instance,SecureGroovyScript::isSandbox); } } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java index e8ae4940..907b4339 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java @@ -26,6 +26,7 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.model.BallColor; +import hudson.model.Descriptor; import hudson.model.PageDecorator; import hudson.security.ACLContext; import jenkins.model.GlobalConfiguration; @@ -68,6 +69,7 @@ import java.util.Stack; import java.util.TreeSet; import java.util.function.Consumer; +import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Pattern; @@ -1004,12 +1006,17 @@ public void setForceSandbox(boolean forceSandbox) { save(); } - + /** + * Flag indicating whether the current system is blocking non sandbox operations for non Admin users. + */ public boolean isForceSandbox() { return forceSandbox; } - //ForceSandbox restrictions does not apply to ADMINISTER users. + /** + * Logic to indicate if the flag {@link #isForceSandbox} applies for the current user.
+ * It does not apply for admin users. + */ public boolean isForceSandboxForCurrentUser() { return forceSandbox && !Jenkins.get().hasPermission(Jenkins.ADMINISTER); } @@ -1335,4 +1342,27 @@ public synchronized JSON clearApprovedClasspathEntries() throws IOException { @Restricted(NoExternalUse.class) @Extension public static class FormValidationPageDecorator extends PageDecorator {} + + /** + * All sandbox checkboxes in the system should confirm their visibility based on this flag.
+ * It depends on the current sandbox value in the affected instance and + * {@link #isForceSandboxForCurrentUser} + * @param isSandbox method handle in the instance class confirming the sandbox current value for the instance. + */ + public static boolean shouldHideSandbox(@CheckForNull T instance, Predicate isSandbox){ + return get().isForceSandboxForCurrentUser() + && (instance == null || isSandbox.test(instance)); + } + + /** + * All describable containing the Sandbox flag should invoke this method before saving.
+ * It will confirm if the current user can persist the information in case the sandbox flag is disabled. + * It depends on {@link #isForceSandboxForCurrentUser} + * In case the current user can't save it will raise a new {@link Descriptor.FormException} + */ + public static void validateSandbox(boolean sandbox) throws Descriptor.FormException{ + if (!sandbox && get().isForceSandboxForCurrentUser()) { + throw new Descriptor.FormException(Messages.ScriptApproval_SandboxCantBeDisabled(), "sandbox"); + } + } } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java index f83b59e2..e25bd110 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java @@ -26,6 +26,8 @@ import org.htmlunit.html.HtmlPage; import org.htmlunit.html.HtmlTextArea; + +import hudson.model.Descriptor; import hudson.model.FreeStyleBuild; import hudson.model.FreeStyleProject; import hudson.model.Item; @@ -209,23 +211,7 @@ public void reload() throws Exception { @Test public void forceSandboxTests() throws Exception { - r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); - - ScriptApproval.get().setForceSandbox(true); - - MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); - mockStrategy.grant(Jenkins.READ).everywhere().to("devel"); - for (Permission p : Item.PERMISSIONS.getPermissions()) { - mockStrategy.grant(p).everywhere().to("devel"); - } - - mockStrategy.grant(Jenkins.READ).everywhere().to("admin"); - mockStrategy.grant(Jenkins.ADMINISTER).everywhere().to("admin"); - for (Permission p : Item.PERMISSIONS.getPermissions()) { - mockStrategy.grant(p).everywhere().to("admin"); - } - - r.jenkins.setAuthorizationStrategy(mockStrategy); + setBasicSecurity(); try (ACLContext ctx = ACL.as(User.getById("devel", true))) { assertTrue(ScriptApproval.get().isForceSandbox()); @@ -299,10 +285,7 @@ public void forceSandboxScriptSignatureException() throws Exception { @Test public void forceSandboxFormValidation() throws Exception { - r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); - r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy(). - grant(Jenkins.READ, Item.READ).everywhere().to("dev"). - grant(Jenkins.ADMINISTER).everywhere().to("admin")); + setBasicSecurity(); try (ACLContext ctx = ACL.as(User.getById("devel", true))) { ScriptApproval.get().setForceSandbox(true); @@ -346,6 +329,98 @@ public void forceSandboxFormValidation() throws Exception { } } + @Test + public void shouldHideSandboxTest() throws Exception { + setBasicSecurity(); + + ScriptApproval.get().setForceSandbox(true); + + SecureGroovyScript testSandboxTrue = new SecureGroovyScript("jenkins.model.Jenkins.instance", true, null); + SecureGroovyScript testSandboxFalse = new SecureGroovyScript("jenkins.model.Jenkins.instance", false, null); + + try (ACLContext ctx = ACL.as(User.getById("devel", true))) { + assertTrue(ScriptApproval.shouldHideSandbox(testSandboxTrue, SecureGroovyScript::isSandbox)); + assertFalse(ScriptApproval.shouldHideSandbox(testSandboxFalse, SecureGroovyScript::isSandbox)); + assertTrue(ScriptApproval.shouldHideSandbox(null, SecureGroovyScript::isSandbox)); + } + + try (ACLContext ctx = ACL.as(User.getById("admin", true))) { + assertFalse(ScriptApproval.shouldHideSandbox(testSandboxTrue, SecureGroovyScript::isSandbox)); + assertFalse(ScriptApproval.shouldHideSandbox(testSandboxFalse, SecureGroovyScript::isSandbox)); + assertFalse(ScriptApproval.shouldHideSandbox(null, SecureGroovyScript::isSandbox)); + } + + ScriptApproval.get().setForceSandbox(false); + + try (ACLContext ctx = ACL.as(User.getById("devel", true))) { + assertFalse(ScriptApproval.shouldHideSandbox(testSandboxTrue, SecureGroovyScript::isSandbox)); + assertFalse(ScriptApproval.shouldHideSandbox(testSandboxFalse, SecureGroovyScript::isSandbox)); + assertFalse(ScriptApproval.shouldHideSandbox(null, SecureGroovyScript::isSandbox)); + } + + try (ACLContext ctx = ACL.as(User.getById("admin", true))) { + assertFalse(ScriptApproval.shouldHideSandbox(testSandboxTrue, SecureGroovyScript::isSandbox)); + assertFalse(ScriptApproval.shouldHideSandbox(testSandboxFalse, SecureGroovyScript::isSandbox)); + assertFalse(ScriptApproval.shouldHideSandbox(null, SecureGroovyScript::isSandbox)); + } + } + + @Test + public void validateSandboxTest() throws Exception { + setBasicSecurity(); + + ScriptApproval.get().setForceSandbox(true); + + try (ACLContext ctx = ACL.as(User.getById("devel", true))) { + ScriptApproval.validateSandbox(true); + assertThrows(Descriptor.FormException.class, + () -> ScriptApproval.validateSandbox(false)); + } + + try (ACLContext ctx = ACL.as(User.getById("admin", true))) { + ScriptApproval.validateSandbox(true); + ScriptApproval.validateSandbox(false); + } + + ScriptApproval.get().setForceSandbox(false); + + try (ACLContext ctx = ACL.as(User.getById("devel", true))) { + ScriptApproval.validateSandbox(true); + ScriptApproval.validateSandbox(false); + } + + try (ACLContext ctx = ACL.as(User.getById("admin", true))) { + ScriptApproval.validateSandbox(true); + ScriptApproval.validateSandbox(false); + } + } + + /** + * Will configure a mock security settings with users: + * Devel: overall Read and write without admin permission + * admin: System administrator + */ + private void setBasicSecurity() + { + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + + ScriptApproval.get().setForceSandbox(true); + + MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); + mockStrategy.grant(Jenkins.READ).everywhere().to("devel"); + for (Permission p : Item.PERMISSIONS.getPermissions()) { + mockStrategy.grant(p).everywhere().to("devel"); + } + + mockStrategy.grant(Jenkins.READ).everywhere().to("admin"); + mockStrategy.grant(Jenkins.ADMINISTER).everywhere().to("admin"); + for (Permission p : Item.PERMISSIONS.getPermissions()) { + mockStrategy.grant(p).everywhere().to("admin"); + } + + r.jenkins.setAuthorizationStrategy(mockStrategy); + } + private Script script(String groovy) { return new Script(groovy); }