diff --git a/README.md b/README.md index 146dbcb..4dd5cd7 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,29 @@ node { ![Sidebar](docs/images/deploy-action.png) +#### Restricting who is shown a link + +A link can be narrowed to particular people with the optional `visibleTo`: + +```groovy +buildAddUrl(title: 'Deploy to PROD', url: '/job/deploy-prod/parambuild/?version=1.2.3', + visibleTo: [users: ['alice'], groups: ['release-managers']]) +``` + +Listing neither users nor groups means no narrowing, and the link behaves as it +always has. When something is listed, the link is hidden from everyone else in +the sidebar and the details bar, **and its URL answers 404** — hiding an icon +is not access control, since the target is in the page source and the link's +path is derivable. + +This only narrows; it cannot widen. Jenkins has already decided who may read +the build before any of this runs. + +**It is not the security boundary for deploying.** The job the link points at +enforces its own permissions -- a `parambuild` URL still requires `Item.BUILD` +on that job. `visibleTo` decides who is shown a shortcut, not who may use what +it points at. + #### Notes on modern Jenkins (plugin 0.2.0+) Since 0.2.0 the button no longer exposes the raw target URL as the action URL. diff --git a/src/main/java/org/jenkinsci/plugins/environmentdashboard/BuildAddUrl.java b/src/main/java/org/jenkinsci/plugins/environmentdashboard/BuildAddUrl.java index b4cfdf3..5b0be43 100644 --- a/src/main/java/org/jenkinsci/plugins/environmentdashboard/BuildAddUrl.java +++ b/src/main/java/org/jenkinsci/plugins/environmentdashboard/BuildAddUrl.java @@ -6,8 +6,10 @@ import hudson.FilePath; import hudson.Launcher; import hudson.Util; +import hudson.model.AbstractDescribableImpl; import hudson.model.AbstractProject; import hudson.model.Action; +import hudson.model.Descriptor; import hudson.model.Run; import hudson.model.TaskListener; import hudson.tasks.BuildStepDescriptor; @@ -16,18 +18,26 @@ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Locale; +import jenkins.model.Jenkins; import jenkins.tasks.SimpleBuildStep; import org.jenkins.ui.icon.IconSpec; import org.jenkinsci.Symbol; import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.DataBoundSetter; import org.kohsuke.stapler.StaplerRequest2; import org.kohsuke.stapler.StaplerResponse2; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.GrantedAuthority; public class BuildAddUrl extends Builder implements SimpleBuildStep { private final String title; private final String url; + private VisibleTo visibleTo; @DataBoundConstructor public BuildAddUrl(String title, String url) { @@ -43,6 +53,15 @@ public String getUrl() { return url; } + public VisibleTo getVisibleTo() { + return visibleTo; + } + + @DataBoundSetter + public void setVisibleTo(VisibleTo visibleTo) { + this.visibleTo = visibleTo; + } + @Override public BuildStepMonitor getRequiredMonitorService() { return BuildStepMonitor.NONE; @@ -56,7 +75,97 @@ public void perform( @NonNull Launcher launcher, @NonNull TaskListener listener) throws InterruptedException, IOException { - run.addAction(new BuildUrlAction(title, url)); + run.addAction(new BuildUrlAction(title, url, visibleTo)); + } + + /** + * Optional narrowing of who is shown a link, by user id or group name. + * + *

Deliberately only principal matching on top of what the authorization + * strategy has already granted. The upstream proposal hand-rolled a + * membership model that ignored the strategy entirely, which both hid the + * link from administrators who were not listed and showed it to listed + * users with no permission on the target. + */ + public static class VisibleTo extends AbstractDescribableImpl { + private List users; + private List groups; + + @DataBoundConstructor + public VisibleTo(List users, List groups) { + this.users = clean(users); + this.groups = clean(groups); + } + + private static List clean(List raw) { + if (raw == null) { + return Collections.emptyList(); + } + List out = new ArrayList<>(raw.size()); + for (String entry : raw) { + String trimmed = Util.fixEmptyAndTrim(entry); + if (trimmed != null) { + out.add(trimmed); + } + } + return Collections.unmodifiableList(out); + } + + private static List split(String text) { + if (Util.fixEmptyAndTrim(text) == null) { + return Collections.emptyList(); + } + return clean(java.util.Arrays.asList(text.split("[,\r\n]"))); + } + + public List getUsers() { + return users; + } + + public List getGroups() { + return groups; + } + + /* + * The form binds through these rather than through the lists directly. + * A textarea submits one string, and binding that straight onto a + * List stores the list's own toString as a single element: a + * round trip of ["alice"] came back as ["[alice]"]. The form rendered + * perfectly well while quietly corrupting what was typed into it, which + * a round-trip test is the only thing that shows. + */ + public String getUsersText() { + return String.join("\n", users); + } + + @DataBoundSetter + public void setUsersText(String usersText) { + this.users = split(usersText); + } + + public String getGroupsText() { + return String.join("\n", groups); + } + + @DataBoundSetter + public void setGroupsText(String groupsText) { + this.groups = split(groupsText); + } + + /** No principals listed means no narrowing, not "nobody". */ + public boolean isEmpty() { + return users.isEmpty() && groups.isEmpty(); + } + + @Extension + @Symbol("visibleTo") + public static class DescriptorImpl extends Descriptor { + @Override + @NonNull + public String getDisplayName() { + return "Visible to"; + } + } } @Extension @@ -88,14 +197,62 @@ public boolean isApplicable(Class t) { public static class BuildUrlAction implements Action, IconSpec { private final String title; private final String url; + private final VisibleTo visibleTo; BuildUrlAction(String title, String url) { + this(title, url, null); + } + + BuildUrlAction(String title, String url, VisibleTo visibleTo) { this.title = title; this.url = url; + this.visibleTo = visibleTo; + } + + public VisibleTo getVisibleTo() { + return visibleTo; + } + + /** + * Whether this link exists at all for the given caller. + * + *

An unset {@code visibleTo} means everyone who can read the build, + * which is what Jenkins has already decided by the time anything here + * runs. When it is set, this narrows that further; it never widens it, + * so it cannot grant anyone access the authorization strategy withheld. + * + *

It is also not the security boundary for deploying. The target of + * the link enforces its own permissions -- a {@code parambuild} URL + * still requires Item.BUILD on the job it points at. This decides who + * is shown a shortcut. + */ + public boolean isVisibleTo(Authentication authentication) { + if (visibleTo == null || visibleTo.isEmpty()) { + return true; + } + if (authentication == null) { + return false; + } + if (visibleTo.getUsers().contains(authentication.getName())) { + return true; + } + for (GrantedAuthority authority : authentication.getAuthorities()) { + if (authority.getAuthority() != null && visibleTo.getGroups().contains(authority.getAuthority())) { + return true; + } + } + return false; + } + + public boolean isVisible() { + return isVisibleTo(Jenkins.getAuthentication2()); } @Override public String getIconFileName() { + if (!isVisible()) { + return null; + } // Hardcoded artifact id: getClass().getPackage().getImplementationTitle() // is unreliable under modern plugin classloaders (may return null). return "/plugin/deploy-dashboard/deploy.png"; @@ -103,7 +260,7 @@ public String getIconFileName() { @Override public String getIconClassName() { - return "symbol-rocket-outline plugin-ionicons-api"; + return isVisible() ? "symbol-rocket-outline plugin-ionicons-api" : null; } @Override @@ -149,10 +306,15 @@ boolean isSafeUrl() { * sidebar and the details bar), it has no side effects, and it is only * reachable through the run's URL, which Jenkins already gates on * Item.READ while resolving the job and the build. + * + *

It does check {@code visibleTo}. Hiding the icon is not access + * control -- the URL is in the page source, and this endpoint has a + * derivable path -- so a link someone is not meant to see answers 404 + * here rather than merely being absent from their sidebar. */ - // lgtm[jenkins/csrf] lgtm[jenkins/no-permission-check] + // lgtm[jenkins/csrf] public void doIndex(StaplerRequest2 req, StaplerResponse2 rsp) throws IOException { - if (!isSafeUrl()) { + if (!isSafeUrl() || !isVisible()) { rsp.sendError(StaplerResponse2.SC_NOT_FOUND); return; } diff --git a/src/main/java/org/jenkinsci/plugins/environmentdashboard/BuildUrlDetailFactory.java b/src/main/java/org/jenkinsci/plugins/environmentdashboard/BuildUrlDetailFactory.java index 27dfc64..912d752 100644 --- a/src/main/java/org/jenkinsci/plugins/environmentdashboard/BuildUrlDetailFactory.java +++ b/src/main/java/org/jenkinsci/plugins/environmentdashboard/BuildUrlDetailFactory.java @@ -29,6 +29,11 @@ public Class type() { @NonNull public List createFor(@NonNull Run target) { return target.getActions(BuildUrlAction.class).stream() + // The details bar is the third place a link surfaces, next to + // the classic sidebar and the redirect endpoint itself. All + // three have to agree, or "hidden" means only "hidden in one + // of the places you might look". + .filter(BuildUrlAction::isVisible) .map(action -> new BuildUrlDetail(target, action)) .collect(Collectors.toList()); } diff --git a/src/main/resources/org/jenkinsci/plugins/environmentdashboard/BuildAddUrl/VisibleTo/config.jelly b/src/main/resources/org/jenkinsci/plugins/environmentdashboard/BuildAddUrl/VisibleTo/config.jelly new file mode 100644 index 0000000..9d53c3c --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/environmentdashboard/BuildAddUrl/VisibleTo/config.jelly @@ -0,0 +1,15 @@ + + + + + + + + + + diff --git a/src/main/resources/org/jenkinsci/plugins/environmentdashboard/BuildAddUrl/config.jelly b/src/main/resources/org/jenkinsci/plugins/environmentdashboard/BuildAddUrl/config.jelly new file mode 100644 index 0000000..19e524f --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/environmentdashboard/BuildAddUrl/config.jelly @@ -0,0 +1,14 @@ + + + + + + + + + + + diff --git a/src/test/java/org/jenkinsci/plugins/environmentdashboard/BuildAddUrlTest.java b/src/test/java/org/jenkinsci/plugins/environmentdashboard/BuildAddUrlTest.java index cc0d4e5..98bd2c8 100644 --- a/src/test/java/org/jenkinsci/plugins/environmentdashboard/BuildAddUrlTest.java +++ b/src/test/java/org/jenkinsci/plugins/environmentdashboard/BuildAddUrlTest.java @@ -3,10 +3,16 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import hudson.model.FreeStyleProject; +import hudson.model.User; +import hudson.security.ACL; +import hudson.security.ACLContext; import java.net.URL; import java.util.List; +import jenkins.model.Jenkins; import jenkins.model.details.Detail; import org.htmlunit.WebRequest; import org.htmlunit.WebResponse; @@ -23,6 +29,27 @@ class BuildAddUrlTest { private static final String TARGET = "/job/app-deploy/parambuild/?imageRegistry=reg.example.com/app&version=1.2.3&revision=abc123&build_url=https://ci.example.com/job/app/1/"; + /** A run whose link is only meant for the listed principals. */ + private static WorkflowRun runWithRestrictedLink(JenkinsRule j, String name, String visibleTo) throws Exception { + WorkflowJob job = j.createProject(WorkflowJob.class, name); + job.setDefinition(new CpsFlowDefinition( + "node { buildAddUrl(title: 'Deploy to PROD', url: '/job/deploy/parambuild/?v=1', visibleTo: [" + + visibleTo + "]) }", + true)); + return j.buildAndAssertSuccess(job); + } + + private static int statusFor(JenkinsRule j, WorkflowRun run, String as) throws Exception { + BuildAddUrl.BuildUrlAction action = run.getAction(BuildAddUrl.BuildUrlAction.class); + try (JenkinsRule.WebClient wc = j.createWebClient()) { + wc.login(as); + wc.getOptions().setRedirectEnabled(false); + wc.getOptions().setThrowExceptionOnFailingStatusCode(false); + return wc.loadWebResponse(new WebRequest(new URL(j.getURL(), run.getUrl() + action.getUrlName() + "/"))) + .getStatusCode(); + } + } + private static WorkflowRun runWithLink(JenkinsRule j, String url) throws Exception { WorkflowJob job = j.createProject(WorkflowJob.class, "app-" + Math.abs(url.hashCode())); job.setDefinition( @@ -82,6 +109,85 @@ void protocolRelativeUrlIsNotRedirected(JenkinsRule j) throws Exception { assertFalse(action.isSafeUrl()); } + @Test + void aRestrictedLinkIsHiddenAndRefusedForEveryoneElse(JenkinsRule j) throws Exception { + // Upstream's version returned null from getIconFileName and stopped + // there. The URL is still in the page source and the endpoint's path is + // derivable, so hiding an icon protects nothing. All three surfaces get + // asserted here, and the redirect is the one that matters. + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + WorkflowRun run = runWithRestrictedLink(j, "restricted", "users: ['alice']"); + BuildAddUrl.BuildUrlAction action = run.getAction(BuildAddUrl.BuildUrlAction.class); + assertNotNull(action); + + try (ACLContext ignored = ACL.as2(User.getById("alice", true).impersonate2())) { + assertTrue(action.isVisible(), "the listed user must see the link"); + assertNotNull(action.getIconFileName()); + assertNotNull(action.getIconClassName()); + assertEquals(1, new BuildUrlDetailFactory().createFor(run).size()); + } + try (ACLContext ignored = ACL.as2(User.getById("bob", true).impersonate2())) { + assertFalse(action.isVisible(), "an unlisted user must not see the link"); + assertNull(action.getIconFileName(), "no sidebar icon"); + assertNull(action.getIconClassName(), "no details-bar icon"); + assertEquals(0, new BuildUrlDetailFactory().createFor(run).size(), "no details-bar entry"); + } + + assertEquals(302, statusFor(j, run, "alice"), "the listed user must still be redirected"); + assertEquals(404, statusFor(j, run, "bob"), "the redirect itself must refuse an unlisted user"); + } + + @Test + void aGroupGrantsVisibility(JenkinsRule j) throws Exception { + org.jvnet.hudson.test.JenkinsRule.DummySecurityRealm realm = j.createDummySecurityRealm(); + realm.addGroups("dave", "release-managers"); + j.jenkins.setSecurityRealm(realm); + + WorkflowRun run = runWithRestrictedLink(j, "grouped", "groups: ['release-managers']"); + BuildAddUrl.BuildUrlAction action = run.getAction(BuildAddUrl.BuildUrlAction.class); + + try (ACLContext ignored = ACL.as2(User.getById("dave", true).impersonate2())) { + assertTrue(action.isVisible(), "a member of the listed group must see the link"); + } + try (ACLContext ignored = ACL.as2(User.getById("carol", true).impersonate2())) { + assertFalse(action.isVisible(), "membership grants it, not merely being a user"); + } + assertEquals(302, statusFor(j, run, "dave")); + assertEquals(404, statusFor(j, run, "carol")); + } + + @Test + void withoutVisibleToTheLinkIsShownToAnyoneWhoCanReadTheBuild(JenkinsRule j) throws Exception { + // The narrowing is opt-in; absent it, nothing changes. + WorkflowRun run = runWithLink(j, TARGET); + BuildAddUrl.BuildUrlAction action = run.getAction(BuildAddUrl.BuildUrlAction.class); + + try (ACLContext ignored = ACL.as2(Jenkins.ANONYMOUS2)) { + assertTrue(action.isVisible(), "an unrestricted link stays visible"); + assertNotNull(action.getIconFileName()); + assertEquals(1, new BuildUrlDetailFactory().createFor(run).size()); + } + } + + @Test + void theFreestyleFormRoundTripsVisibleTo(JenkinsRule j) throws Exception { + // The step's form is new, and a form that renders but does not bind is + // worse than none: it silently discards what was typed into it. + FreeStyleProject job = j.createFreeStyleProject("form-roundtrip"); + BuildAddUrl step = new BuildAddUrl("Deploy to PROD", "/job/deploy/parambuild/?v=1"); + step.setVisibleTo(new BuildAddUrl.VisibleTo(List.of("alice"), List.of("release-managers"))); + job.getBuildersList().add(step); + + j.configRoundtrip(job); + + BuildAddUrl saved = job.getBuildersList().get(BuildAddUrl.class); + assertNotNull(saved, "the build step must survive a configuration round trip"); + assertEquals("Deploy to PROD", saved.getTitle()); + assertNotNull(saved.getVisibleTo(), "visibleTo must survive the round trip"); + assertEquals(List.of("alice"), saved.getVisibleTo().getUsers()); + assertEquals(List.of("release-managers"), saved.getVisibleTo().getGroups()); + } + @Test void detailFactoryExposesLinkOnDetailsBar(JenkinsRule j) throws Exception { WorkflowRun run = runWithLink(j, TARGET);