diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java index 8cf85f81e951f9c3a67dcf5e0c28681643b2daab..c1dcb27f0603536f80de6a95f534b12992695cc5 100644 --- a/core/src/main/java/hudson/Functions.java +++ b/core/src/main/java/hudson/Functions.java @@ -1172,6 +1172,31 @@ public class Functions { ac.checkAnyPermission(permissions); } + /** + * This version is so that the 'checkAnyPermission' on {@code layout.jelly} + * degrades gracefully if "it" is not an {@link AccessControlled} object. + * Otherwise it will perform no check and that problem is hard to notice. + */ + public static void checkAnyPermission(Object object, Permission[] permissions) throws IOException, ServletException { + if (permissions == null || permissions.length == 0) { + return; + } + + if (object instanceof AccessControlled) + checkAnyPermission((AccessControlled) object, permissions); + else { + List ancs = Stapler.getCurrentRequest().getAncestors(); + for(Ancestor anc : Iterators.reverse(ancs)) { + Object o = anc.getObject(); + if (o instanceof AccessControlled) { + checkAnyPermission((AccessControlled) o, permissions); + return; + } + } + checkAnyPermission(Jenkins.get(), permissions); + } + } + private static class Tag implements Comparable { double ordinal; String hierarchy; diff --git a/test/src/test/java/hudson/security/ACLTest.java b/test/src/test/java/hudson/security/ACLTest.java index fb5906441025eb75716b102da08c8b1d5a20e034..5b9e5a26022d20700c939fb8904776782f56a09b 100644 --- a/test/src/test/java/hudson/security/ACLTest.java +++ b/test/src/test/java/hudson/security/ACLTest.java @@ -24,8 +24,10 @@ package hudson.security; +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; import hudson.model.FreeStyleProject; import hudson.model.Item; +import hudson.model.UnprotectedRootAction; import hudson.model.User; import java.util.Collection; import java.util.Collections; @@ -39,6 +41,9 @@ import org.junit.rules.ExpectedException; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.MockAuthorizationStrategy; +import org.jvnet.hudson.test.TestExtension; + +import javax.annotation.CheckForNull; public class ACLTest { @@ -121,6 +126,29 @@ public class ACLTest { r.jenkins.getACL().checkAnyPermission(); } + @Test + @Issue("JENKINS-61465") + public void checkAnyPermissionOnNonAccessControlled() throws Exception { + expectedException = ExpectedException.none(); + + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() + .grant(Jenkins.READ).everywhere().toEveryone()); + + JenkinsRule.WebClient wc = r.createWebClient(); + try { + wc.goTo("either"); + fail(); + } catch (FailingHttpStatusCodeException ex) { + assertEquals(403, ex.getStatusCode()); + } + + r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() + .grant(Jenkins.ADMINISTER).everywhere().toEveryone()); + + wc.goTo("either"); // expected to work + } + private static class DoNotBotherMe extends AuthorizationStrategy { @Override @@ -140,4 +168,26 @@ public class ACLTest { } + @TestExtension + public static class EitherPermission implements UnprotectedRootAction { + + @CheckForNull + @Override + public String getIconFileName() { + return null; + } + + @CheckForNull + @Override + public String getDisplayName() { + return null; + } + + @CheckForNull + @Override + public String getUrlName() { + return "either"; + } + } + } diff --git a/test/src/test/resources/hudson/security/ACLTest/EitherPermission/index.jelly b/test/src/test/resources/hudson/security/ACLTest/EitherPermission/index.jelly new file mode 100644 index 0000000000000000000000000000000000000000..cda2f1d493589c972343c556e5b4f123ed6dd30b --- /dev/null +++ b/test/src/test/resources/hudson/security/ACLTest/EitherPermission/index.jelly @@ -0,0 +1,39 @@ + + + + + + + +
+
+
+

Hello, world!

+
+
+
+
+
+