Skip to content

Commit

Permalink
[SECURITY-2804] limit support actions to user with ADMINISTER karma only
Browse files Browse the repository at this point in the history
Signed-off-by: Olivier Lamy <olamy@apache.org>
  • Loading branch information
olamy authored and daniel-beck committed Nov 9, 2022
1 parent abaff01 commit 9b7a1d4
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@
@Extension
@ExportedBean
public class SupportAction implements RootAction, StaplerProxy {

/**
* @deprecated see {@link SupportPlugin#CREATE_BUNDLE}
*/
@Deprecated
public static final Permission CREATE_BUNDLE = SupportPlugin.CREATE_BUNDLE;
/**
* Our logger (retain an instance ref to avoid classloader leaks).
Expand All @@ -88,7 +93,7 @@ public class SupportAction implements RootAction, StaplerProxy {
@Override
@Restricted(NoExternalUse.class)
public Object getTarget() {
Jenkins.get().checkPermission(CREATE_BUNDLE);
Jenkins.get().checkPermission(Jenkins.ADMINISTER);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ protected void printUsageSummary(PrintStream stderr) {

@Override
protected int run() throws Exception {
Jenkins.get().checkPermission(SupportPlugin.CREATE_BUNDLE);
Jenkins.get().checkPermission(Jenkins.ADMINISTER);
Set<Component> selected = new HashSet<>();

// JENKINS-63722: If "Master" or "Agents" are unselected, show a warning and add the components to the list
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ public class SupportPlugin extends Plugin {
public static final PermissionGroup SUPPORT_PERMISSIONS =
new PermissionGroup(SupportPlugin.class, Messages._SupportPlugin_PermissionGroup());

/**
* @deprecated not used anymore as the usage has now been limited to {@link Jenkins#ADMINISTER}
*/
@Deprecated
public static final Permission CREATE_BUNDLE =
new Permission(SUPPORT_PERMISSIONS, "DownloadBundle", Messages._SupportPlugin_CreateBundle(),
Jenkins.ADMINISTER, PermissionScope.JENKINS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout">
<l:task icon="${h.getIconFilePath(action)}" title="${action.displayName}"
href="${h.getActionUrl(it.url,action)}"
permission="${action.CREATE_BUNDLE}"/>
permission="${app.ADMINISTER}"/>
</j:jelly>
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:l="/lib/layout"
xmlns:f="/lib/form">
<l:layout title="${it.actionTitleText}" norefresh="true" permission="${it.CREATE_BUNDLE}">
<l:layout title="${it.actionTitleText}" norefresh="true" permission="${app.ADMINISTER}">
<st:include page="sidepanel.jelly" it="${app}"/>
<l:main-panel>
<h1>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public void deleteExistingBundleWithoutPermissionWillFail() throws IOException {
Path bundle = createFakeSupportBundle();
assertTrue(Files.exists(bundle));
WebResponse response = deleteBundle(bundle.getFileName().toString(), "user");
assertThat(response.getContentAsString(), containsString(String.format("user is missing the %s/%s permission", SupportAction.CREATE_BUNDLE.group.title, SupportAction.CREATE_BUNDLE.name)));
assertThat(response.getContentAsString(), containsString(String.format("user is missing the %s/%s permission", Jenkins.ADMINISTER.group.title, Jenkins.ADMINISTER.name)));
assertThat(response.getStatusCode(), equalTo(403));
}

Expand Down

0 comments on commit 9b7a1d4

Please sign in to comment.