diff --git a/core/src/main/resources/lib/form/dropdownDescriptorSelector.jelly b/core/src/main/resources/lib/form/dropdownDescriptorSelector.jelly index 084e47717153..359c12d982b6 100644 --- a/core/src/main/resources/lib/form/dropdownDescriptorSelector.jelly +++ b/core/src/main/resources/lib/form/dropdownDescriptorSelector.jelly @@ -47,6 +47,9 @@ THE SOFTWARE. Config fragments from descriptors are rendered lazily by default, which means variables seen in the caller aren't visible to them. This attribute allows you to nominate additional variables and their values to be captured for descriptors. + Note that "readOnlyMode" is always captured automatically, so callers do not + need to (and should not need to) list it explicitly for read-only mode to be + propagated into the lazily rendered descriptor fragment. @@ -61,7 +64,7 @@ THE SOFTWARE. + lazy="descriptor,it,readOnlyMode,${capture}"> diff --git a/test/src/test/java/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest.java b/test/src/test/java/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest.java new file mode 100644 index 000000000000..c719c3649579 --- /dev/null +++ b/test/src/test/java/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest.java @@ -0,0 +1,108 @@ +package hudson.security; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import edu.umd.cs.findbugs.annotations.NonNull; +import hudson.model.Descriptor; +import java.util.Collection; +import java.util.Collections; +import jenkins.model.Jenkins; +import org.htmlunit.WebClientUtil; +import org.htmlunit.html.DomElement; +import org.htmlunit.html.HtmlPage; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +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 org.jvnet.hudson.test.junit.jupiter.WithJenkins; +import org.kohsuke.stapler.DataBoundConstructor; + +/** + * Reproduces JENKINS-12548 on the "Manage Jenkins > Security" page + * ({@code hudson/security/GlobalSecurityConfiguration/index.groovy}), a + * second, architecturally distinct rendering path (Groovy view, reachable + * with only {@link Jenkins#SYSTEM_READ}) for the same underlying bug as + * {@link jenkins.scm.SCMCheckoutStrategyReadOnlyModeTest}: the "Authorization" + * {@code f.dropdownDescriptorSelector} lazily loads non-selected + * {@link AuthorizationStrategy} fragments via {@code l:renderOnDemand}, which + * only forwards variables named in its {@code capture} attribute. Fixed once, + * for every caller, in {@code lib/form/dropdownDescriptorSelector.jelly} + * itself rather than on this page individually. + */ +@WithJenkins +class GlobalSecurityConfigurationReadOnlyModeTest { + + private JenkinsRule j; + + @BeforeEach + void setUp(JenkinsRule rule) { + j = rule; + } + + @Issue("JENKINS-12548") + @Test + void lazilyLoadedAuthorizationStrategyFragmentIsDisabledForSystemReadViewer() throws Exception { + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() + .grant(Jenkins.ADMINISTER).everywhere().to("admin") + .grant(Jenkins.READ, Jenkins.SYSTEM_READ).everywhere().to("viewer")); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.withBasicCredentials("viewer"); + HtmlPage page = wc.goTo("configureSecurity"); + + // Triggers the same client-side fetch normally fired when the user picks + // a non-default option from the "Authorization" dropdown. + page.executeJavaScript( + "document.getElementsBySelector('.render-on-demand').forEach(function(e) { renderOnDemand(e); })"); + WebClientUtil.waitForJSExec(wc); + + DomElement authorizationBlock = page.querySelector("div[name='authorizationStrategy']"); + assertNotNull(authorizationBlock, "expected the Authorization dropdown block to be rendered"); + + assertNull(authorizationBlock.querySelector("input[name='_.value']"), + "the lazily-loaded FieldAuthorizationStrategy fragment must not expose an editable 'value' field" + + " to a viewer with only Jenkins.SYSTEM_READ (no Jenkins.ADMINISTER)"); + assertNotNull(authorizationBlock.querySelector(".jenkins-not-applicable"), + "the read-only viewer should instead see the field rendered as the standard N/A read-only placeholder"); + } + + public static class FieldAuthorizationStrategy extends AuthorizationStrategy { + + private final String value; + + @SuppressWarnings("checkstyle:redundantmodifier") + @DataBoundConstructor + public FieldAuthorizationStrategy(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + + @NonNull + @Override + public ACL getRootACL() { + return ACL.lambda2((a, p) -> false); + } + + @NonNull + @Override + public Collection getGroups() { + return Collections.emptySet(); + } + + @TestExtension + public static class DescriptorImpl extends Descriptor { + @NonNull + @Override + public String getDisplayName() { + return "FieldAuthorizationStrategy"; + } + } + } +} diff --git a/test/src/test/java/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest.java b/test/src/test/java/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest.java new file mode 100644 index 000000000000..dbb273f8a158 --- /dev/null +++ b/test/src/test/java/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest.java @@ -0,0 +1,98 @@ +package jenkins.scm; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import hudson.model.AbstractProject; +import hudson.model.FreeStyleProject; +import hudson.model.Item; +import jenkins.model.Jenkins; +import org.htmlunit.WebClientUtil; +import org.htmlunit.html.DomElement; +import org.htmlunit.html.HtmlPage; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +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 org.jvnet.hudson.test.junit.jupiter.WithJenkins; +import org.kohsuke.stapler.DataBoundConstructor; + +/** + * Reproduces JENKINS-12548: the SCM Checkout Strategy dropdown in + * {@code config-scm.jelly} renders non-selected descriptors lazily via + * {@code l:renderOnDemand}, which only forwards variables named in its + * {@code capture} attribute. Before the fix in + * {@code lib/form/dropdownDescriptorSelector.jelly} (which now always + * captures {@code readOnlyMode}, alongside the already-always-captured + * {@code descriptor}/{@code it}), a viewer with {@link Item#EXTENDED_READ} + * but not {@link Item#CONFIGURE} got an editable field once that fragment + * was lazily loaded. + */ +@WithJenkins +class SCMCheckoutStrategyReadOnlyModeTest { + + private JenkinsRule j; + + @BeforeEach + void setUp(JenkinsRule rule) { + j = rule; + } + + @Issue("JENKINS-12548") + @Test + void lazilyLoadedStrategyFragmentIsDisabledForReadOnlyViewer() throws Exception { + FreeStyleProject p = j.createFreeStyleProject("p"); + // Leave the default strategy selected so FieldSCMCheckoutStrategy's + // config.jelly is the one deferred via l:renderOnDemand. + + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() + .grant(Jenkins.ADMINISTER).everywhere().to("admin") + .grant(Jenkins.READ, Item.READ, Item.EXTENDED_READ).everywhere().to("viewer")); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.withBasicCredentials("viewer"); + HtmlPage page = wc.goTo("job/" + p.getName() + "/configure"); + + // Triggers the same client-side fetch normally fired when the user picks + // a non-default option from the "SCM Checkout Strategy" dropdown. + page.executeJavaScript( + "document.getElementsBySelector('.render-on-demand').forEach(function(e) { renderOnDemand(e); })"); + WebClientUtil.waitForJSExec(wc); + + DomElement strategyBlock = page.querySelector("div[name='scmCheckoutStrategy']"); + assertNotNull(strategyBlock, "expected the SCM Checkout Strategy dropdown block to be rendered"); + + assertNull(strategyBlock.querySelector("input[name='_.value']"), + "the lazily-loaded FieldSCMCheckoutStrategy fragment must not expose an editable 'value' field" + + " to a read-only (Item.EXTENDED_READ, no Item.CONFIGURE) viewer"); + assertNotNull(strategyBlock.querySelector(".jenkins-not-applicable"), + "the read-only viewer should instead see the field rendered as the standard N/A read-only placeholder"); + } + + @SuppressWarnings("rawtypes") + public static class FieldSCMCheckoutStrategy extends SCMCheckoutStrategy { + + private final String value; + + @SuppressWarnings("checkstyle:redundantmodifier") + @DataBoundConstructor + public FieldSCMCheckoutStrategy(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + + @TestExtension + public static class DescriptorImpl extends SCMCheckoutStrategyDescriptor { + @Override + public boolean isApplicable(AbstractProject project) { + return true; + } + } + } +} diff --git a/test/src/test/java/lib/form/DropdownDescriptorSelectorTest.java b/test/src/test/java/lib/form/DropdownDescriptorSelectorTest.java new file mode 100644 index 000000000000..508e0fe012cd --- /dev/null +++ b/test/src/test/java/lib/form/DropdownDescriptorSelectorTest.java @@ -0,0 +1,114 @@ +package lib.form; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import hudson.model.Describable; +import hudson.model.Descriptor; +import hudson.model.InvisibleAction; +import hudson.model.RootAction; +import java.util.Objects; +import jenkins.model.Jenkins; +import org.htmlunit.WebClientUtil; +import org.htmlunit.html.DomElement; +import org.htmlunit.html.HtmlPage; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.TestExtension; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; +import org.kohsuke.stapler.DataBoundConstructor; + +/** + * Tests that {@code f:dropdownDescriptorSelector} always captures {@code readOnlyMode} into + * the lazily-rendered ({@code l:renderOnDemand}) fragment of a non-selected descriptor, without + * the caller having to opt in via the taglib's {@code capture} attribute. This exercises the + * taglib mechanism directly (see {@code test1.jelly}, which does not pass {@code capture}), + * independently of any specific page's permission model. + */ +@WithJenkins +class DropdownDescriptorSelectorTest { + + private JenkinsRule j; + + @BeforeEach + void setUp(JenkinsRule rule) { + j = rule; + } + + @Issue("JENKINS-12548") + @Test + void readOnlyModeIsCapturedWithoutCallerOptingIn() throws Exception { + HtmlPage p = j.createWebClient().goTo("self/test1"); + + // Triggers the same client-side fetch normally fired when the user picks + // a non-default option from the dropdown. + p.executeJavaScript( + "document.getElementsBySelector('.render-on-demand').forEach(function(e) { renderOnDemand(e); })"); + WebClientUtil.waitForJSExec(p.getWebClient()); + + DomElement fruitBlock = p.querySelector("div[name='fruit']"); + assertNotNull(fruitBlock, "expected the dropdown block to be rendered"); + + assertNull(fruitBlock.querySelector("input[name='_.value']"), + "the lazily-loaded, non-selected descriptor fragment must not expose an editable 'value' field" + + " when readOnlyMode is true, even though test1.jelly's f:dropdownDescriptorSelector" + + " does not pass capture=\"readOnlyMode\""); + assertNotNull(fruitBlock.querySelector(".jenkins-not-applicable"), + "read-only mode should instead render the standard N/A read-only placeholder"); + } + + public abstract static class Fruit implements Describable { + private final String value; + + protected Fruit(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + } + + public static class Apple extends Fruit { + @SuppressWarnings("checkstyle:redundantmodifier") + @DataBoundConstructor + public Apple(String value) { + super(value); + } + + @TestExtension + public static class DescriptorImpl extends Descriptor {} + } + + public static class Banana extends Fruit { + @SuppressWarnings("checkstyle:redundantmodifier") + @DataBoundConstructor + public Banana(String value) { + super(value); + } + + @TestExtension + public static class DescriptorImpl extends Descriptor {} + } + + @TestExtension + public static final class RootActionImpl extends InvisibleAction implements Describable, RootAction { + + public Fruit fruit = new Apple("seed"); + + @Override + public Descriptor getDescriptor() { + return Objects.requireNonNull(Jenkins.get().getDescriptorByType(DescriptorImpl.class)); + } + + @TestExtension + public static final class DescriptorImpl extends Descriptor {} + + @Override + public String getUrlName() { + return "self"; + } + } +} diff --git a/test/src/test/resources/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest/FieldAuthorizationStrategy/config.jelly b/test/src/test/resources/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest/FieldAuthorizationStrategy/config.jelly new file mode 100644 index 000000000000..69a00c9b4117 --- /dev/null +++ b/test/src/test/resources/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest/FieldAuthorizationStrategy/config.jelly @@ -0,0 +1,7 @@ + + + + + + + diff --git a/test/src/test/resources/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest/FieldSCMCheckoutStrategy/config.jelly b/test/src/test/resources/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest/FieldSCMCheckoutStrategy/config.jelly new file mode 100644 index 000000000000..69a00c9b4117 --- /dev/null +++ b/test/src/test/resources/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest/FieldSCMCheckoutStrategy/config.jelly @@ -0,0 +1,7 @@ + + + + + + + diff --git a/test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/Fruit/config.jelly b/test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/Fruit/config.jelly new file mode 100644 index 000000000000..69a00c9b4117 --- /dev/null +++ b/test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/Fruit/config.jelly @@ -0,0 +1,7 @@ + + + + + + + diff --git a/test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/RootActionImpl/test1.jelly b/test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/RootActionImpl/test1.jelly new file mode 100644 index 000000000000..72bb54575eab --- /dev/null +++ b/test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/RootActionImpl/test1.jelly @@ -0,0 +1,16 @@ + + + + + + + + + + + + + + +