diff --git a/CHANGELOG.md b/CHANGELOG.md index 14263a2..b3f4c5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,17 @@ Changelog ----- +### 0.12.1 + +Release date: Apr 30, 2018 + +* [JENKINS-49744](https://issues.jenkins-ci.org/browse/JENKINS-49744) - +Users with `Manage Ownership` permissions were unable to change Foler ownership from CLI/REST API without `Jenkins/Administer` persiossion. +* [JENKINS-50807](https://issues.jenkins-ci.org/browse/JENKINS-50807) - +Add missing implementations for [OwnershipHelperLocator](https://jenkins.io/doc/developer/extensions/ownership/#ownershiphelperlocator) +extension point. +Now the API can be used to retrieve ownership info for any object. + ### 0.12.0 Release date: Feb 26, 2018 diff --git a/pom.xml b/pom.xml index 7e67173..614698f 100644 --- a/pom.xml +++ b/pom.xml @@ -3,16 +3,16 @@ org.jenkins-ci.plugins plugin - 2.37 + 3.5 com.synopsys.jenkinsci ownership - 0.12.1-SNAPSHOT + 0.12.2-SNAPSHOT Job and Node ownership plugin hpi Provides explicit ownership of jobs and nodes - https://wiki.jenkins-ci.org/display/JENKINS/Ownership+Plugin + https://wiki.jenkins.io/display/JENKINS/Ownership+Plugin @@ -27,6 +27,7 @@ 7 UTF-8 1.14 + Max diff --git a/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/OwnershipDescription.java b/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/OwnershipDescription.java index 6e6ddfb..218d338 100644 --- a/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/OwnershipDescription.java +++ b/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/OwnershipDescription.java @@ -23,16 +23,14 @@ */ package com.synopsys.arc.jenkins.plugins.ownership; -import com.synopsys.arc.jenkins.plugins.ownership.jobs.JobOwnerJobProperty; +import com.synopsys.arc.jenkins.plugins.ownership.util.AbstractOwnershipHelper; import com.synopsys.arc.jenkins.plugins.ownership.util.IdStrategyComparator; import com.synopsys.arc.jenkins.plugins.ownership.util.OwnershipDescriptionHelper; import com.synopsys.arc.jenkins.plugins.ownership.nodes.OwnerNodeProperty; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.Util; -import hudson.model.Computer; import hudson.model.Descriptor; -import hudson.model.Job; -import hudson.model.Node; +import hudson.model.ModelObject; import hudson.model.User; import hudson.security.ACL; import hudson.security.AccessControlled; @@ -44,6 +42,8 @@ import java.util.Objects; import java.util.Set; import java.util.TreeSet; +import java.util.logging.Level; +import java.util.logging.Logger; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -51,6 +51,7 @@ import net.sf.json.JSONObject; import org.acegisecurity.AccessDeniedException; import org.acegisecurity.Authentication; +import org.jenkinsci.plugins.ownership.model.OwnershipHelperLocator; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted; import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerRequest; @@ -62,6 +63,9 @@ * @since 0.0.3 */ public class OwnershipDescription implements Serializable { + + private static final Logger LOGGER = Logger.getLogger(OwnershipDescription.class.getName()); + /** * Disabled description, which means that ownership is disabled */ @@ -392,29 +396,22 @@ private void checkUnsecuredConfiguration() throws ObjectStreamException { StaplerRequest request = Stapler.getCurrentRequest(); if (request != null) { AccessControlled context = request.findAncestorObject(AccessControlled.class); - if (context instanceof Job) { - Job job = (Job)context; - JobOwnerJobProperty existing = job.getProperty(JobOwnerJobProperty.class); - if (existing == null || !Objects.equals(existing.getOwnership(), this)) { - throwIfMissingPermission(job, OwnershipPlugin.MANAGE_ITEMS_OWNERSHIP); - } - return; - } else if (context instanceof Computer) { - Node node = ((Computer)context).getNode(); - if (node != null) { - OwnerNodeProperty existing = node.getNodeProperties().get(OwnerNodeProperty.class); - if (existing == null || !Objects.equals(existing.getOwnership(), this)) { - throwIfMissingPermission(node, OwnershipPlugin.MANAGE_SLAVES_OWNERSHIP); + if (context != null) { + final AbstractOwnershipHelper helper = OwnershipHelperLocator.locate(context); + if (helper != null) { + final OwnershipDescription d = helper.getOwnershipDescription(context); + if (!helper.hasLocallyDefinedOwnership(context) || !Objects.equals(d, this)) { + throwIfMissingPermission(context, helper.getRequiredPermission()); } return; + } else { + LOGGER.log(Level.WARNING, "Cannot locate OwnershipHelperClass for object {0}. " + + "Jenkins.ADMINISTER permissions will be required to change ownership", context); } - } else if (context instanceof Node) { - Node node = ((Node)context); - OwnerNodeProperty existing = node.getNodeProperties().get(OwnerNodeProperty.class); - if (existing == null || !Objects.equals(existing.getOwnership(), this)) { - throwIfMissingPermission(node, OwnershipPlugin.MANAGE_SLAVES_OWNERSHIP); - } - return; + } else { + //TODO: maybe it should rejected, because there is no use-cases for it so far AFAIK + LOGGER.log(Level.WARNING, "Ownership Description is used outside the object context. " + + "Jenkins.ADMINISTER permissions will be required to change ownership"); } } // We don't know what object this OwnershipDescription belongs to, so we require Overall/Administer permissions. @@ -422,11 +419,22 @@ private void checkUnsecuredConfiguration() throws ObjectStreamException { throwIfMissingPermission(Jenkins.getActiveInstance(), Jenkins.ADMINISTER); } - private void throwIfMissingPermission(AccessControlled context, Permission permission) throws ObjectStreamException { + private void throwIfMissingPermission(@Nonnull AccessControlled context, Permission permission) throws ObjectStreamException { try { context.checkPermission(permission); } catch (AccessDeniedException e) { - throw new InvalidObjectException(e.getMessage()); + final String name; + if (context instanceof ModelObject) { + name = ((ModelObject)context).getDisplayName(); + } else { + name = context.toString(); + } + + InvalidObjectException ex = new InvalidObjectException( + String.format("Cannot modify permissions of %s of type %s: %s", name, + context.getClass(), e.getMessage())); + ex.addSuppressed(e); + throw ex; } } diff --git a/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/jobs/JobOwnerHelper.java b/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/jobs/JobOwnerHelper.java index 4020e9c..4c878c9 100644 --- a/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/jobs/JobOwnerHelper.java +++ b/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/jobs/JobOwnerHelper.java @@ -43,9 +43,13 @@ import java.util.Collection; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; + +import hudson.security.Permission; import org.jenkinsci.plugins.ownership.model.OwnershipHelperLocator; import org.jenkinsci.plugins.ownership.model.OwnershipInfo; import org.jenkinsci.plugins.ownership.model.jobs.JobOwnershipDescriptionSource; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Helper for Jobs Ownership. @@ -95,6 +99,16 @@ public static boolean isUserExists(@Nonnull String userIdOrFullName) { return getOwnershipInfo(job).getDescription(); } + @Override + public Permission getRequiredPermission() { + return OwnershipPlugin.MANAGE_ITEMS_OWNERSHIP; + } + + @Override + public boolean hasLocallyDefinedOwnership(@Nonnull Job job) { + return getOwnerProperty(job) != null; + } + @Override public OwnershipInfo getOwnershipInfo(Job job) { JobOwnerJobProperty prop = getOwnerProperty(job); @@ -199,6 +213,7 @@ public String getItemURL(Job item) { } @Extension + @Restricted(NoExternalUse.class) public static class LocatorImpl extends OwnershipHelperLocator> { @Override diff --git a/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/ComputerOwnerHelper.java b/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/ComputerOwnerHelper.java index 41978c3..c849526 100644 --- a/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/ComputerOwnerHelper.java +++ b/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/ComputerOwnerHelper.java @@ -24,7 +24,9 @@ package com.synopsys.arc.jenkins.plugins.ownership.nodes; import com.synopsys.arc.jenkins.plugins.ownership.OwnershipDescription; +import com.synopsys.arc.jenkins.plugins.ownership.OwnershipPlugin; import com.synopsys.arc.jenkins.plugins.ownership.util.AbstractOwnershipHelper; +import hudson.Extension; import hudson.model.Computer; import hudson.model.Node; import hudson.model.User; @@ -33,7 +35,12 @@ import java.util.Collections; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; + +import hudson.security.Permission; +import org.jenkinsci.plugins.ownership.model.OwnershipHelperLocator; import org.jenkinsci.plugins.ownership.model.OwnershipInfo; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Provides ownership helper for {@link Computer}. @@ -83,6 +90,21 @@ public static void setOwnership(@Nonnull Computer computer, NodeOwnerHelper.setOwnership(node, descr); } + @Override + public Permission getRequiredPermission() { + return OwnershipPlugin.MANAGE_SLAVES_OWNERSHIP; + } + + @Override + public boolean hasLocallyDefinedOwnership(@Nonnull Computer computer) { + Node node = computer.getNode(); + if (node == null) { + // Node is not defined => permission is detached + return false; + } + return NodeOwnerHelper.Instance.hasLocallyDefinedOwnership(node); + } + @Override public String getItemTypeName(Computer item) { return "computer"; @@ -98,4 +120,17 @@ public String getItemURL(Computer item) { //TODO: Absolute URL return item.getUrl(); } + + @Extension + @Restricted(NoExternalUse.class) + public static class LocatorImpl extends OwnershipHelperLocator { + + @Override + public AbstractOwnershipHelper findHelper(Object item) { + if (item instanceof Computer) { + return INSTANCE; + } + return null; + } + } } diff --git a/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/NodeOwnerHelper.java b/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/NodeOwnerHelper.java index 57ec2a5..dc59c2a 100644 --- a/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/NodeOwnerHelper.java +++ b/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/NodeOwnerHelper.java @@ -29,7 +29,9 @@ import com.synopsys.arc.jenkins.plugins.ownership.util.UserCollectionFilter; import com.synopsys.arc.jenkins.plugins.ownership.util.userFilters.AccessRightsFilter; import com.synopsys.arc.jenkins.plugins.ownership.util.userFilters.IUserFilter; +import hudson.Extension; import hudson.model.Computer; +import hudson.model.Job; import hudson.model.Node; import hudson.model.User; @@ -37,8 +39,13 @@ import java.util.Collection; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; + +import hudson.security.Permission; +import org.jenkinsci.plugins.ownership.model.OwnershipHelperLocator; import org.jenkinsci.plugins.ownership.model.OwnershipInfo; import org.jenkinsci.plugins.ownership.model.nodes.NodeOwnershipDescriptionSource; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Provides helper for Node owner. @@ -83,7 +90,17 @@ public OwnershipInfo getOwnershipInfo(Node item) { return prop != null ? new OwnershipInfo(OwnershipDescription.DISABLED_DESCR, new NodeOwnershipDescriptionSource(item)) : OwnershipInfo.DISABLED_INFO; } - + + @Override + public Permission getRequiredPermission() { + return OwnershipPlugin.MANAGE_SLAVES_OWNERSHIP; + } + + @Override + public boolean hasLocallyDefinedOwnership(@Nonnull Node node) { + return getOwnerProperty(node) != null; + } + @Override public Collection getPossibleOwners(Node item) { if (OwnershipPlugin.getInstance().isRequiresConfigureRights()) { @@ -126,4 +143,17 @@ public String getItemURL(Node item) { Computer c = item.toComputer(); return c != null ? ComputerOwnerHelper.INSTANCE.getItemURL(c) : null; } + + @Extension + @Restricted(NoExternalUse.class) + public static class LocatorImpl extends OwnershipHelperLocator { + + @Override + public AbstractOwnershipHelper findHelper(Object item) { + if (item instanceof Node) { + return Instance; + } + return null; + } + } } diff --git a/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/NodeOwnerPropertyHelper.java b/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/NodeOwnerPropertyHelper.java index aa08def..b8b3c9a 100644 --- a/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/NodeOwnerPropertyHelper.java +++ b/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/NodeOwnerPropertyHelper.java @@ -27,14 +27,21 @@ import com.synopsys.arc.jenkins.plugins.ownership.OwnershipPlugin; import com.synopsys.arc.jenkins.plugins.ownership.util.AbstractOwnershipHelper; import com.synopsys.arc.jenkins.plugins.ownership.util.UserCollectionFilter; +import hudson.Extension; +import hudson.model.Job; import hudson.model.Node; import hudson.model.User; +import hudson.security.Permission; import hudson.slaves.NodeProperty; import java.util.Collection; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; + +import org.jenkinsci.plugins.ownership.model.OwnershipHelperLocator; import org.jenkinsci.plugins.ownership.model.OwnershipInfo; import org.jenkinsci.plugins.ownership.model.nodes.NodeOwnershipDescriptionSource; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Provides helper for Node owner @@ -94,6 +101,17 @@ public Collection getPossibleOwners(NodeProperty item) { return null; } + @Override + public Permission getRequiredPermission() { + return OwnershipPlugin.MANAGE_SLAVES_OWNERSHIP; + } + + @Override + public boolean hasLocallyDefinedOwnership(@Nonnull NodeProperty item) { + // Self-defined + return true; + } + @Override public String getItemTypeName(NodeProperty item) { return NodeOwnerHelper.ITEM_TYPE_NAME; @@ -109,5 +127,18 @@ public String getItemDisplayName(NodeProperty item) { public String getItemURL(NodeProperty item) { Node node = getNode(item); return node != null ? NodeOwnerHelper.Instance.getItemURL(node) : null; - } + } + + @Extension + @Restricted(NoExternalUse.class) + public static class LocatorImpl extends OwnershipHelperLocator { + + @Override + public AbstractOwnershipHelper findHelper(Object item) { + if (item instanceof NodeProperty) { + return Instance; + } + return null; + } + } } diff --git a/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/util/AbstractOwnershipHelper.java b/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/util/AbstractOwnershipHelper.java index 82dfcc5..3983889 100644 --- a/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/util/AbstractOwnershipHelper.java +++ b/src/main/java/com/synopsys/arc/jenkins/plugins/ownership/util/AbstractOwnershipHelper.java @@ -32,6 +32,9 @@ import java.util.Collections; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; + +import hudson.security.Permission; +import jenkins.model.Jenkins; import org.jenkinsci.plugins.ownership.model.OwnershipInfo; /** @@ -96,7 +99,7 @@ public boolean isDisplayOwnershipSummaryBox(@Nonnull TObjectType item) { return true; } - + /** * Gets ownership info of the requested item. * @param item Item to be described @@ -106,8 +109,28 @@ public boolean isDisplayOwnershipSummaryBox(@Nonnull TObjectType item) { */ @Nonnull public abstract OwnershipInfo getOwnershipInfo(@Nonnull TObjectType item); - + public boolean hasItemSpecificPermission(@Nonnull TObjectType item, String sid, Permission p) { return false; } + + /** + * Gets permission required to manage ownership for the item. + * {@link Jenkins#ADMINISTER} by default if not overridden. + * @return Permission which is needed to change ownership. + * @since TODO + */ + @Nonnull + public Permission getRequiredPermission() { + return Jenkins.ADMINISTER; + } + + /** + * Check if the objeck has locally defined ownership info. + * @param item Item + * @return {@code true} if the object has ownership defined locally. + * {@code false} will be returned otherwise, even if ownership is inherited. + * @since TODO + */ + public boolean hasLocallyDefinedOwnership(@Nonnull TObjectType item) { return false; } } diff --git a/src/main/java/org/jenkinsci/plugins/ownership/model/folders/FolderOwnershipHelper.java b/src/main/java/org/jenkinsci/plugins/ownership/model/folders/FolderOwnershipHelper.java index 5530367..da7e7a5 100755 --- a/src/main/java/org/jenkinsci/plugins/ownership/model/folders/FolderOwnershipHelper.java +++ b/src/main/java/org/jenkinsci/plugins/ownership/model/folders/FolderOwnershipHelper.java @@ -42,8 +42,12 @@ import java.util.Collection; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; + +import hudson.security.Permission; import org.jenkinsci.plugins.ownership.model.OwnershipHelperLocator; import org.jenkinsci.plugins.ownership.model.OwnershipInfo; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Integration with Folders plugin. @@ -93,6 +97,17 @@ public OwnershipDescription getOwnershipDescription(AbstractFolder item) { return getOwnershipInfo(item).getDescription(); } + @Nonnull + @Override + public Permission getRequiredPermission() { + return OwnershipPlugin.MANAGE_ITEMS_OWNERSHIP; + } + + @Override + public boolean hasLocallyDefinedOwnership(@Nonnull AbstractFolder folder) { + return getOwnerProperty(folder) != null; + } + @Override public OwnershipInfo getOwnershipInfo(AbstractFolder item) { if (item == null) { // Handle renames, etc. @@ -201,6 +216,7 @@ public String getItemURL(AbstractFolder item) { } @Extension(optional = true) + @Restricted(NoExternalUse.class) public static class LocatorImpl extends OwnershipHelperLocator> { @Override diff --git a/src/main/java/org/jenkinsci/plugins/ownership/model/runs/RunOwnershipHelper.java b/src/main/java/org/jenkinsci/plugins/ownership/model/runs/RunOwnershipHelper.java index 6face9b..3b390fb 100644 --- a/src/main/java/org/jenkinsci/plugins/ownership/model/runs/RunOwnershipHelper.java +++ b/src/main/java/org/jenkinsci/plugins/ownership/model/runs/RunOwnershipHelper.java @@ -30,6 +30,7 @@ import com.synopsys.arc.jenkins.plugins.ownership.nodes.OwnerNodeProperty; import com.synopsys.arc.jenkins.plugins.ownership.util.AbstractOwnershipHelper; import com.synopsys.arc.jenkins.plugins.ownership.util.UserStringFormatter; +import hudson.Extension; import hudson.model.AbstractBuild; import hudson.model.BuildListener; import hudson.model.Node; @@ -37,7 +38,12 @@ import java.util.Map; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; + +import hudson.security.Permission; +import org.jenkinsci.plugins.ownership.model.OwnershipHelperLocator; import org.jenkinsci.plugins.ownership.model.OwnershipInfo; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Helper for {@link Run} ownership management. @@ -46,10 +52,10 @@ */ public class RunOwnershipHelper extends AbstractOwnershipHelper { - private static final RunOwnershipHelper instance = new RunOwnershipHelper(); + private static final RunOwnershipHelper INSTANCE = new RunOwnershipHelper(); public static RunOwnershipHelper getInstance() { - return instance; + return INSTANCE; } @Override @@ -76,7 +82,13 @@ public OwnershipDescription getOwnershipDescription(Run item) { public OwnershipInfo getOwnershipInfo(Run item) { return JobOwnerHelper.Instance.getOwnershipInfo(item.getParent()); } - + + @Override + public Permission getRequiredPermission() { + // Runs do not have separate permission management logic now, so we rely on Items + return OwnershipPlugin.MANAGE_ITEMS_OWNERSHIP; + } + /** * Environment setup according to wrapper configurations. * @param build Input build @@ -143,4 +155,17 @@ public boolean isDisplayOwnershipSummaryBox(Run item) { return super.isDisplayOwnershipSummaryBox(item) && !OwnershipPlugin.getInstance().getConfiguration().getDisplayOptions().isHideRunOwnership(); } + + @Extension + @Restricted(NoExternalUse.class) + public static class LocatorImpl extends OwnershipHelperLocator { + + @Override + public AbstractOwnershipHelper findHelper(Object item) { + if (item instanceof Run) { + return INSTANCE; + } + return null; + } + } } diff --git a/src/test/java/com/synopsys/arc/jenkins/plugins/ownership/jobs/JobOwnerJobPropertyTest.java b/src/test/java/com/synopsys/arc/jenkins/plugins/ownership/jobs/JobOwnerJobPropertyTest.java index 196f80b..c917ebf 100644 --- a/src/test/java/com/synopsys/arc/jenkins/plugins/ownership/jobs/JobOwnerJobPropertyTest.java +++ b/src/test/java/com/synopsys/arc/jenkins/plugins/ownership/jobs/JobOwnerJobPropertyTest.java @@ -40,6 +40,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.JenkinsRule.WebClient; import org.jvnet.hudson.test.MockAuthorizationStrategy; @@ -70,6 +71,7 @@ public void setupSecurity() { } @Test + @Issue("SECURITY-498") public void changeOwnerViaPost() throws Exception { FreeStyleProject p = r.createFreeStyleProject(); p.getProperty(JobOwnerJobProperty.class).setOwnershipDescription(new OwnershipDescription(true, "admin", null)); @@ -100,6 +102,7 @@ public void changeOwnerViaPost() throws Exception { } @Test + @Issue("SECURITY-498") public void changeOwnerViaCLI() throws Exception { FreeStyleProject p = r.createFreeStyleProject(); p.getProperty(JobOwnerJobProperty.class).setOwnershipDescription(new OwnershipDescription(true, "admin", null)); diff --git a/src/test/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/OwnerNodePropertyTest.java b/src/test/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/OwnerNodePropertyTest.java index e987d52..ebf503d 100644 --- a/src/test/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/OwnerNodePropertyTest.java +++ b/src/test/java/com/synopsys/arc/jenkins/plugins/ownership/nodes/OwnerNodePropertyTest.java @@ -38,6 +38,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.MockAuthorizationStrategy; @@ -67,6 +68,7 @@ public void setupSecurity() { } @Test + @Issue("SECURITY-498") public void changeOwnerViaPost() throws Exception { String nodeName; // Computer#updateByXml replaces the existing node with a new instance, so we always need to look up the current instance. String nodeUrl; @@ -103,6 +105,7 @@ public void changeOwnerViaPost() throws Exception { } @Test + @Issue("SECURITY-498") public void changeOwnerViaCLI() throws Exception { String nodeName; { diff --git a/src/test/java/org/jenkinsci/plugins/ownership/folders/FolderOwnershipPropertyTest.java b/src/test/java/org/jenkinsci/plugins/ownership/folders/FolderOwnershipPropertyTest.java new file mode 100644 index 0000000..8a952e2 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/ownership/folders/FolderOwnershipPropertyTest.java @@ -0,0 +1,174 @@ +/* + * The MIT License + * + * Copyright 2018 CloudBees, Inc., Oleg Nenashev + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package org.jenkinsci.plugins.ownership.folders; + +import com.cloudbees.hudson.plugins.folder.Folder; +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; +import com.gargoylesoftware.htmlunit.HttpMethod; +import com.gargoylesoftware.htmlunit.WebRequest; +import com.synopsys.arc.jenkins.plugins.ownership.OwnershipDescription; +import com.synopsys.arc.jenkins.plugins.ownership.util.AbstractOwnershipHelper; +import hudson.cli.CLICommandInvoker; +import hudson.cli.UpdateJobCommand; +import hudson.model.Item; +import jenkins.model.Jenkins; +import org.jenkinsci.plugins.ownership.model.OwnershipHelperLocator; +import org.jenkinsci.plugins.ownership.model.folders.FolderOwnershipHelper; +import org.jenkinsci.plugins.ownership.model.folders.FolderOwnershipProperty; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.For; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.JenkinsRule.WebClient; +import org.jvnet.hudson.test.MockAuthorizationStrategy; + +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; + +import static hudson.cli.CLICommandInvoker.Matcher.succeededSilently; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +// TODO: DRY, merge with JobOwnerJobHelper once helper#setOwnership() is a non-static method +@For(FolderOwnershipProperty.class) +public class FolderOwnershipPropertyTest { + + @Rule + public JenkinsRule r = new JenkinsRule(); + + private Folder p; + private AbstractOwnershipHelper ownershipHelper; + + @Before + public void setupSecurity() { + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + MockAuthorizationStrategy mas = new MockAuthorizationStrategy(); + mas.grant(Jenkins.ADMINISTER) // Implies MANAGE_ITEMS_OWNERSHIP. + .everywhere() + .to("admin"); + mas.grant(Item.CONFIGURE, Item.READ, Jenkins.READ) + .everywhere() + .to("non-admin"); + r.jenkins.setAuthorizationStrategy(mas); + } + + @Before + public void initFolder() throws Exception { + // Unique project name + p = r.createProject(Folder.class, "test" + r.jenkins.getItems().size()); + ownershipHelper = OwnershipHelperLocator.locate(p); + if (ownershipHelper == null) { + throw new AssertionError("Cannot locate ownership helper for " + p + " of type " + p.getClass()); + } + } + + @Test + @Issue("JENKINS-49744") + public void changeOwnerViaPost() throws Exception { + FolderOwnershipHelper.setOwnership(p, + new OwnershipDescription(true, "admin", null)); + + WebClient wc = r.createWebClient(); + wc.login("non-admin", "non-admin"); + WebRequest req = new WebRequest(wc.createCrumbedUrl(String.format("%sconfig.xml", p.getUrl())), HttpMethod.POST); + req.setAdditionalHeader("Content-Type", "application/xml"); + req.setRequestBody(getItemXml("admin")); + wc.getPage(req); + assertThat("Users should be able to configure Folder when ownership is unchanged", + ownershipHelper.getOwner(p), is(equalTo("admin"))); + + try { + wc.login("non-admin", "non-admin"); + req.setRequestBody(getItemXml("non-admin")); + wc.getPage(req); + } catch (FailingHttpStatusCodeException e) { + // fine + } + assertThat(ownershipHelper.getOwner(p), is(equalTo("admin"))); + + wc.login("admin", "admin"); + req.setRequestBody(getItemXml("non-admin")); + wc.getPage(req); + assertThat("Users with Manage Ownership/Jobs permissions should be able to change ownership", + ownershipHelper.getOwner(p), is(equalTo("non-admin"))); + } + + @Test + @Issue("JENKINS-49744") + public void changeOwnerViaCLI() throws Exception { + FolderOwnershipHelper.setOwnership(p, + new OwnershipDescription(true, "admin", null)); + + CLICommandInvoker command = new CLICommandInvoker(r, new UpdateJobCommand()) + .asUser("non-admin") + .withArgs(p.getFullName()) + .withStdin(getItemXmlAsStream("admin")); + assertThat(ownershipHelper.getOwner(p), is(equalTo("admin"))); + + command.asUser("admin") + .withArgs(p.getFullName()) + .withStdin(getItemXmlAsStream("non-admin")); + assertThat("Users with Overall/Administer permissions should be able to configure jobs via CLI", + command.invoke(), succeededSilently()); + assertThat(ownershipHelper.getOwner(p), is(equalTo("non-admin"))); + } + + private String getItemXml(String ownerSid) { + return String.format(FOLDER_XML_TEMPLATE, ownerSid); + } + + private InputStream getItemXmlAsStream(String ownerSid) { + return new ByteArrayInputStream(getItemXml(ownerSid).getBytes(StandardCharsets.UTF_8)); + } + + private static final String FOLDER_XML_TEMPLATE = + "" + + "" + + " " + + " " + + " " + + " true" + + " %s" + + " " + + " " + + " " + + " " + + " \n" + + " \n" + + " \n" + + " All\n" + + " false\n" + + " false\n" + + " \n" + + " \n" + + " \n" + + " " + + ""; + +}