Skip to content

Commit 8f8fbd2

Browse files
jglickjglick
authored
Merge pull request #623 from jglick/requesterAuthentication
Removing `requesterAuthentication`
2 parents 6a6bf31 + 43af22c commit 8f8fbd2

File tree

6 files changed

+82
-111
lines changed

6 files changed

+82
-111
lines changed

‎src/main/java/com/cloudbees/jenkins/support/SupportAction.java

+5-25
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@
3434
import hudson.model.Api;
3535
import hudson.model.Failure;
3636
import hudson.model.RootAction;
37-
import hudson.security.ACL;
38-
import hudson.security.ACLContext;
3937
import hudson.security.Permission;
4038
import jakarta.servlet.ServletException;
4139
import jakarta.servlet.ServletOutputStream;
@@ -374,12 +372,7 @@ public HttpRedirect doGenerateBundleAsync(StaplerRequest2 req, StaplerResponse2
374372
}
375373
try (FileOutputStream fileOutputStream =
376374
new FileOutputStream(new File(outputDir.toString(), SYNC_SUPPORT_BUNDLE))) {
377-
SupportPlugin.setRequesterAuthentication(Jenkins.getAuthentication2());
378-
try {
379-
SupportPlugin.writeBundleForSyncComponents(fileOutputStream, syncComponent);
380-
} finally {
381-
SupportPlugin.clearRequesterAuthentication();
382-
}
375+
SupportPlugin.writeBundleForSyncComponents(fileOutputStream, syncComponent);
383376
} finally {
384377
logger.fine("Processing support bundle sunc completed");
385378
}
@@ -472,18 +465,10 @@ private void prepareBundle(StaplerResponse2 rsp, List<Component> components) thr
472465
rsp.addHeader("Content-Disposition", "inline; filename=" + BundleFileName.generate() + ";");
473466
final ServletOutputStream servletOutputStream = rsp.getOutputStream();
474467
try {
475-
SupportPlugin.setRequesterAuthentication(Jenkins.getAuthentication2());
476-
try {
477-
try (ACLContext ignored = ACL.as2(ACL.SYSTEM2)) {
478-
SupportPlugin.writeBundle(servletOutputStream, components);
479-
} catch (IOException e) {
480-
logger.log(Level.FINE, e.getMessage(), e);
481-
}
482-
} finally {
483-
SupportPlugin.clearRequesterAuthentication();
484-
}
485-
} finally {
468+
SupportPlugin.writeBundle(servletOutputStream, components);
486469
logger.fine("Response completed");
470+
} catch (IOException e) {
471+
logger.log(Level.FINE, e.getMessage(), e);
487472
}
488473
}
489474

@@ -554,12 +539,7 @@ protected void compute() throws Exception {
554539

555540
try (FileOutputStream fileOutputStream =
556541
new FileOutputStream(new File(outputDir.toString(), supportBundleName))) {
557-
SupportPlugin.setRequesterAuthentication(Jenkins.getAuthentication2());
558-
try {
559-
SupportPlugin.writeBundle(fileOutputStream, components, this::progress, outputDir);
560-
} finally {
561-
SupportPlugin.clearRequesterAuthentication();
562-
}
542+
SupportPlugin.writeBundle(fileOutputStream, components, this::progress, outputDir);
563543
} finally {
564544
logger.fine("Processing support bundle async completed");
565545
}

‎src/main/java/com/cloudbees/jenkins/support/SupportCommand.java

+6-15
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@
3131
import hudson.Extension;
3232
import hudson.cli.CLICommand;
3333
import hudson.remoting.RemoteOutputStream;
34-
import hudson.security.ACL;
35-
import hudson.security.ACLContext;
3634
import java.io.FileOutputStream;
3735
import java.io.IOException;
3836
import java.io.OutputStream;
@@ -93,20 +91,13 @@ protected int run() throws Exception {
9391
selected.add(c);
9492
}
9593
}
96-
SupportPlugin.setRequesterAuthentication(Jenkins.getAuthentication2());
97-
try {
98-
try (ACLContext ignored = ACL.as2(ACL.SYSTEM2)) {
99-
OutputStream os;
100-
if (channel != null) { // Remoting mode
101-
os = channel.call(new SaveBundle(BundleFileName.generate()));
102-
} else { // redirect output to a ZIP file yourself
103-
os = new CloseProofOutputStream(stdout);
104-
}
105-
SupportPlugin.writeBundle(os, new ArrayList<>(selected));
106-
}
107-
} finally {
108-
SupportPlugin.clearRequesterAuthentication();
94+
OutputStream os;
95+
if (channel != null) { // Remoting mode
96+
os = channel.call(new SaveBundle(BundleFileName.generate()));
97+
} else { // redirect output to a ZIP file yourself
98+
os = new CloseProofOutputStream(stdout);
10999
}
100+
SupportPlugin.writeBundle(os, new ArrayList<>(selected));
110101
return 0;
111102
}
112103

‎src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java

+5-11
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,6 @@ public class SupportPlugin extends Plugin {
163163
Jenkins.ADMINISTER,
164164
PermissionScope.JENKINS);
165165

166-
private static final ThreadLocal<Authentication> requesterAuthentication = new InheritableThreadLocal<>();
167166
private static final AtomicLong nextBundleWrite = new AtomicLong(Long.MIN_VALUE);
168167
private static final Logger logger = Logger.getLogger(SupportPlugin.class.getName());
169168
public static final String SUPPORT_DIRECTORY_NAME = "support";
@@ -239,16 +238,12 @@ public static File getLogsDirectory() {
239238
return new File(SafeTimerTask.getLogsRoot(), SUPPORT_DIRECTORY_NAME);
240239
}
241240

241+
/**
242+
* @deprecated Check permissions directly.
243+
*/
244+
@Deprecated
242245
public static Authentication getRequesterAuthentication() {
243-
return requesterAuthentication.get();
244-
}
245-
246-
public static void setRequesterAuthentication(Authentication authentication) {
247-
requesterAuthentication.set(authentication);
248-
}
249-
250-
public static void clearRequesterAuthentication() {
251-
requesterAuthentication.remove();
246+
return Jenkins.getAuthentication2();
252247
}
253248

254249
public void setSupportProvider(SupportProvider supportProvider) throws IOException {
@@ -1066,7 +1061,6 @@ protected synchronized void doRun() throws Exception {
10661061
thread.setName(String.format(
10671062
"%s periodic bundle generator: since %s",
10681063
SupportPlugin.class.getSimpleName(), new Date()));
1069-
clearRequesterAuthentication();
10701064
try (ACLContext ignored = ACL.as2(ACL.SYSTEM2)) {
10711065
File bundleDir = getRootDirectory();
10721066
if (!bundleDir.exists()) {

‎src/main/java/com/cloudbees/jenkins/support/actions/SupportObjectAction.java

+13-21
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
import hudson.model.Action;
1414
import hudson.model.Descriptor;
1515
import hudson.model.Saveable;
16-
import hudson.security.ACL;
17-
import hudson.security.ACLContext;
1816
import hudson.util.DescribableList;
1917
import jakarta.servlet.ServletException;
2018
import jakarta.servlet.http.HttpServletResponse;
@@ -139,26 +137,20 @@ public final void doGenerateAndDownload(StaplerRequest2 req, StaplerResponse2 rs
139137
"Content-Disposition", "inline; filename=" + BundleFileName.generate(getBundleNameQualifier()) + ";");
140138

141139
try {
142-
SupportPlugin.setRequesterAuthentication(Jenkins.getAuthentication2());
143-
try (ACLContext old = ACL.as2(ACL.SYSTEM2)) {
144-
SupportPlugin.writeBundle(
145-
rsp.getOutputStream(),
146-
components,
147-
new ComponentVisitor() {
148-
@Override
149-
public <C extends Component> void visit(Container container, C component) {
150-
((ObjectComponent<T>) component).addContents(container, object);
151-
}
152-
},
153-
null,
154-
true);
155-
} catch (IOException e) {
156-
LOGGER.log(Level.WARNING, e.getMessage(), e);
157-
} finally {
158-
SupportPlugin.clearRequesterAuthentication();
159-
}
160-
} finally {
140+
SupportPlugin.writeBundle(
141+
rsp.getOutputStream(),
142+
components,
143+
new ComponentVisitor() {
144+
@Override
145+
public <C extends Component> void visit(Container container, C component) {
146+
((ObjectComponent<T>) component).addContents(container, object);
147+
}
148+
},
149+
null,
150+
true);
161151
LOGGER.fine("Response completed");
152+
} catch (IOException e) {
153+
LOGGER.log(Level.WARNING, e.getMessage(), e);
162154
}
163155
}
164156

‎src/main/java/com/cloudbees/jenkins/support/impl/AboutUser.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,19 @@
11
package com.cloudbees.jenkins.support.impl;
22

3-
import com.cloudbees.jenkins.support.SupportPlugin;
43
import com.cloudbees.jenkins.support.api.Component;
54
import com.cloudbees.jenkins.support.api.Container;
65
import com.cloudbees.jenkins.support.api.PrefilteredPrintedContent;
76
import com.cloudbees.jenkins.support.filter.ContentFilter;
87
import edu.umd.cs.findbugs.annotations.NonNull;
98
import hudson.Extension;
9+
import hudson.security.ACL;
1010
import hudson.security.Permission;
1111
import java.io.IOException;
1212
import java.io.PrintWriter;
1313
import java.util.Collection;
1414
import java.util.Collections;
1515
import java.util.Set;
16+
import jenkins.model.Jenkins;
1617
import org.springframework.security.core.Authentication;
1718
import org.springframework.security.core.GrantedAuthority;
1819

@@ -36,8 +37,8 @@ public Set<Permission> getRequiredPermissions() {
3637

3738
@Override
3839
public void addContents(@NonNull Container result) {
39-
final Authentication authentication = SupportPlugin.getRequesterAuthentication();
40-
if (authentication != null) {
40+
final Authentication authentication = Jenkins.getAuthentication2();
41+
if (!authentication.equals(ACL.SYSTEM2)) {
4142
result.add(new PrefilteredPrintedContent("user.md") {
4243
@Override
4344
protected void printTo(PrintWriter out, ContentFilter filter) throws IOException {

‎src/test/java/com/cloudbees/jenkins/support/CheckFilterTest.java

+49-36
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.cloudbees.jenkins.support;
22

3+
import static org.hamcrest.MatcherAssert.assertThat;
4+
import static org.hamcrest.Matchers.empty;
35
import static org.junit.Assert.fail;
46

57
import com.cloudbees.jenkins.support.api.Component;
@@ -21,11 +23,13 @@
2123
import com.cloudbees.jenkins.support.impl.UpdateCenter;
2224
import hudson.EnvVars;
2325
import hudson.ExtensionList;
26+
import hudson.Functions;
2427
import hudson.model.FreeStyleBuild;
2528
import hudson.model.FreeStyleProject;
2629
import hudson.model.User;
2730
import hudson.model.labels.LabelAtom;
2831
import hudson.model.queue.QueueTaskFuture;
32+
import hudson.security.ACL;
2933
import java.io.BufferedInputStream;
3034
import java.io.ByteArrayOutputStream;
3135
import java.io.File;
@@ -88,25 +92,31 @@ public void checkFilterTest() throws Exception {
8892
// Generate the filtered words
8993
checker.generateFilteredWords();
9094

91-
// Check the components are filtered correctly
92-
assertComponent(
93-
Arrays.asList(
94-
AboutJenkins.class,
95-
AboutUser.class,
96-
AgentsConfigFile.class,
97-
BuildQueue.class,
98-
ConfigFileComponent.class,
99-
DumpExportTable.class,
100-
EnvironmentVariables.class,
101-
JVMProcessSystemMetricsContents.Agents.class,
102-
JVMProcessSystemMetricsContents.Master.class,
103-
SystemConfiguration.class,
104-
NetworkInterfaces.class,
105-
NodeMonitors.class,
106-
UpdateCenter.class,
107-
SystemProperties.class,
108-
ThreadDumps.class),
109-
checker);
95+
j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
96+
try (var ignored = ACL.as(User.getById("admin", true))) {
97+
// Check the components are filtered correctly
98+
assertComponent(
99+
Arrays.asList(
100+
AboutJenkins.class,
101+
AboutUser.class,
102+
AgentsConfigFile.class,
103+
BuildQueue.class,
104+
ConfigFileComponent.class,
105+
DumpExportTable.class,
106+
EnvironmentVariables.class,
107+
JVMProcessSystemMetricsContents.Agents.class,
108+
JVMProcessSystemMetricsContents.Master.class,
109+
SystemConfiguration.class,
110+
NetworkInterfaces.class,
111+
NodeMonitors.class,
112+
UpdateCenter.class,
113+
SystemProperties.class,
114+
ThreadDumps.class),
115+
checker);
116+
}
117+
if (!Functions.isWindows()) { // some entries for procfs skipped on Windows
118+
assertThat(checker.unchecked.stream().map(f -> f.filePattern).toList(), empty());
119+
}
110120

111121
// Cancel the job running
112122
build.cancel(true);
@@ -219,24 +229,25 @@ private static String getFirstEnvVar() {
219229

220230
private static class FileChecker {
221231
private Set<FileToCheck> fileSet = new HashSet<>();
232+
private Set<FileToCheck> unchecked = new HashSet<>();
222233
private Set<String> words = new HashSet<>();
223234

224235
private FileChecker(Jenkins jenkins) throws UnknownHostException {
225236
fileSet.add(of("manifest.md", "about", false));
226237
fileSet.add(of("about.md", "b", false));
227-
fileSet.add(of("items.md", "jobs", false));
238+
// fileSet.add(of("items.md", "jobs", false));
228239
// checksum.md5 is not generated in tests
229240

230241
// AboutJenkins -> nodes.md
231242
// The agent node name should be filtered
232243
fileSet.add(of("nodes.md", AGENT_NAME, true));
233244
// AboutUser
234-
fileSet.add(of("user.md", "anonymous", true));
245+
fileSet.add(of("user.md", "admin", true));
235246

236-
fileSet.add(of("node-monitors.md", AGENT_NAME, true));
247+
// fileSet.add(of("node-monitors.md", AGENT_NAME, true));
237248

238-
fileSet.add(of("admin-monitors.md", "diagnostics", false));
239-
fileSet.add(of("admin-monitors.md", "Family", false));
249+
// fileSet.add(of("admin-monitors.md", "diagnostics", false));
250+
// fileSet.add(of("admin-monitors.md", "Family", false));
240251

241252
fileSet.add(of("nodes/slave/*/config.xml", AGENT_NAME, true));
242253

@@ -293,23 +304,23 @@ private FileChecker(Jenkins jenkins) throws UnknownHostException {
293304
// system-uptime.txt
294305
// net/rpc/nfs.txt
295306
// net/rpc/nfsd.txt
296-
fileSet.add(of("nodes/master/proc/swaps.txt", "cpu", false));
297-
fileSet.add(of("nodes/slave/*/proc/swaps.txt", "cpu", false));
307+
// fileSet.add(of("nodes/master/proc/swaps.txt", "cpu", false));
308+
// fileSet.add(of("nodes/slave/*/proc/swaps.txt", "cpu", false));
298309

299-
fileSet.add(of("nodes/master/proc/cpuinfo.txt", "cpu", false));
300-
fileSet.add(of("nodes/slave/*/proc/cpuinfo.txt", "cpu", false));
310+
// fileSet.add(of("nodes/master/proc/cpuinfo.txt", "cpu", false));
311+
// fileSet.add(of("nodes/slave/*/proc/cpuinfo.txt", "cpu", false));
301312

302-
fileSet.add(of("nodes/master/proc/mounts.txt", "cpu", false));
303-
fileSet.add(of("nodes/slave/*/proc/mounts.txt", "cpu", false));
313+
// fileSet.add(of("nodes/master/proc/mounts.txt", "cpu", false));
314+
// fileSet.add(of("nodes/slave/*/proc/mounts.txt", "cpu", false));
304315

305-
fileSet.add(of("nodes/master/proc/system-uptime.txt", "cpu", false));
306-
fileSet.add(of("nodes/slave/*/proc/system-uptime.txt", "cpu", false));
316+
// fileSet.add(of("nodes/master/proc/system-uptime.txt", "cpu", false));
317+
// fileSet.add(of("nodes/slave/*/proc/system-uptime.txt", "cpu", false));
307318

308-
fileSet.add(of("nodes/master/proc/net/rpc/nfs.txt", "cpu", false));
309-
fileSet.add(of("nodes/slave/*/proc/net/rpc/nfs.txt", "cpu", false));
319+
// fileSet.add(of("nodes/master/proc/net/rpc/nfs.txt", "cpu", false));
320+
// fileSet.add(of("nodes/slave/*/proc/net/rpc/nfs.txt", "cpu", false));
310321

311-
fileSet.add(of("nodes/master/proc/net/rpc/nfsd.txt", "cpu", false));
312-
fileSet.add(of("nodes/slave/*/proc/net/rpc/nfsd.txt", "cpu", false));
322+
// fileSet.add(of("nodes/master/proc/net/rpc/nfsd.txt", "cpu", false));
323+
// fileSet.add(of("nodes/slave/*/proc/net/rpc/nfsd.txt", "cpu", false));
313324

314325
// NetworkInterfaces --> nodes/master/networkInterface.md, nodes/slave/*/networkInterface.md
315326
String anIP = getInetAddress();
@@ -333,6 +344,7 @@ private FileChecker(Jenkins jenkins) throws UnknownHostException {
333344
// ThreadDumps --> nodes/slave/*/thread-dump.txt, nodes/master/thread-dump.txt
334345
fileSet.add(of("nodes/master/thread-dump.txt", "runnable", true));
335346
fileSet.add(of("nodes/slave/*/thread-dump.txt", "runnable", true));
347+
unchecked.addAll(fileSet);
336348
}
337349

338350
private String getUpdateCenterURL(Jenkins jenkins) {
@@ -393,6 +405,7 @@ private void check(String file, String content) {
393405
for (FileToCheck value : fileSet) {
394406
// If there is a filePattern to check that matches this entry of the bundle, we check
395407
if (value.match(file)) {
408+
unchecked.remove(value);
396409
if (content == null) {
397410
fail(String.format("Error checking the file %s because its content was null", file));
398411
} else {

0 commit comments

Comments
 (0)