Skip to content

Commit 15b07e6

Browse files
committed
fix tests
rename method in DoubleLaunchChecker so the monitor doesn't disable itself
1 parent 6f6b28b commit 15b07e6

File tree

4 files changed

+40
-14
lines changed

4 files changed

+40
-14
lines changed

core/src/main/java/hudson/model/AdministrativeMonitor.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,12 +127,14 @@ public final String getSearchUrl() {
127127
* Mark this monitor as disabled, to prevent this from showing up in the UI.
128128
*
129129
* <p>
130-
* This is a per-user setting if security is enabled.
130+
* This is a per-user setting if security is enabled. This means that it should not be used to disable
131+
* the monitor programmatically.
131132
* </p>
132133
*/
133134
public void disable(boolean value) throws IOException {
134135
AbstractCIBase jenkins = Jenkins.get();
135136
User user = User.current();
137+
136138
if (user != null) {
137139
AdministrativeMonitorsProperty property = AdministrativeMonitorsProperty.get(user);
138140
property.disableMonitor(id, value);

core/src/main/java/hudson/util/DoubleLaunchChecker.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ protected void execute() {
108108
}
109109

110110
try {
111-
Files.writeString(Util.fileToPath(timestampFile), getId(), Charset.defaultCharset());
111+
Files.writeString(Util.fileToPath(timestampFile), getPid(), Charset.defaultCharset());
112112
lastWriteTime = timestampFile.lastModified();
113113
} catch (IOException e) {
114114
LOGGER.log(Level.FINE, null, e);
@@ -120,7 +120,7 @@ protected void execute() {
120120
/**
121121
* Figures out a string that identifies this instance of Hudson.
122122
*/
123-
private String getId() {
123+
private String getPid() {
124124
return Long.toString(ProcessHandle.current().pid());
125125
}
126126

core/src/main/java/jenkins/user/AdministrativeMonitorsProperty.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ public AdministrativeMonitorsProperty() {
4040

4141
@Override
4242
public UserProperty reconfigure(StaplerRequest2 req, JSONObject json) throws Descriptor.FormException {
43+
if (json == null) {
44+
return this;
45+
}
4346
JSONArray monitors = json.optJSONArray("administrativeMonitor");
4447
synchronized (dismissedMonitors) {
4548
for (AdministrativeMonitor am : AdministrativeMonitor.all()) {

test/src/test/java/hudson/diagnosis/HudsonHomeDiskUsageMonitorTest.java

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
import static org.junit.jupiter.api.Assertions.assertTrue;
77

88
import hudson.model.User;
9+
import hudson.security.ACL;
10+
import hudson.security.ACLContext;
911
import hudson.security.GlobalMatrixAuthorizationStrategy;
1012
import java.io.IOException;
1113
import java.net.HttpURLConnection;
@@ -19,6 +21,8 @@
1921
import org.htmlunit.html.HtmlForm;
2022
import org.htmlunit.html.HtmlPage;
2123
import org.htmlunit.util.NameValuePair;
24+
import org.jenkinsci.plugins.matrixauth.AuthorizationType;
25+
import org.jenkinsci.plugins.matrixauth.PermissionEntry;
2226
import org.junit.jupiter.api.BeforeEach;
2327
import org.junit.jupiter.api.Test;
2428
import org.jvnet.hudson.test.Issue;
@@ -45,17 +49,27 @@ void flow() throws Exception {
4549
HudsonHomeDiskUsageMonitor mon = HudsonHomeDiskUsageMonitor.get();
4650
mon.activated = true;
4751

52+
JenkinsRule.DummySecurityRealm realm = j.createDummySecurityRealm();
53+
realm.addGroups("administrator", "admins");
54+
j.jenkins.setSecurityRealm(realm);
55+
GlobalMatrixAuthorizationStrategy auth = new GlobalMatrixAuthorizationStrategy();
56+
auth.add(Jenkins.ADMINISTER, new PermissionEntry(AuthorizationType.GROUP, "admins"));
57+
User administrator = User.getById("administrator", true);
58+
j.submit(getForm(mon, administrator), "yes");
59+
4860
// clicking yes should take us to somewhere
49-
j.submit(getForm(mon), "yes");
50-
assertTrue(mon.isEnabled());
61+
try (ACLContext c = ACL.as(administrator)) {
62+
assertTrue(mon.isEnabled());
63+
}
5164

5265
// now dismiss
53-
// submit(getForm(mon),"no"); TODO: figure out why this test is fragile
54-
mon.doAct("no");
55-
assertFalse(mon.isEnabled());
66+
j.submit(getForm(mon, administrator),"no");
67+
try (ACLContext c = ACL.as(administrator)) {
68+
assertFalse(mon.isEnabled());
69+
}
5670

5771
// and make sure it's gone
58-
assertThrows(ElementNotFoundException.class, () -> getForm(mon));
72+
assertThrows(ElementNotFoundException.class, () -> getForm(mon, administrator));
5973
}
6074

6175
@Issue("SECURITY-371")
@@ -70,8 +84,8 @@ void noAccessForNonAdmin() throws Exception {
7084
realm.addGroups("bob", "users");
7185
j.jenkins.setSecurityRealm(realm);
7286
GlobalMatrixAuthorizationStrategy auth = new GlobalMatrixAuthorizationStrategy();
73-
auth.add(Jenkins.ADMINISTER, "admins");
74-
auth.add(Jenkins.READ, "users");
87+
auth.add(Jenkins.ADMINISTER, new PermissionEntry(AuthorizationType.GROUP, "admins"));
88+
auth.add(Jenkins.READ, new PermissionEntry(AuthorizationType.GROUP, "users"));
7589
j.jenkins.setAuthorizationStrategy(auth);
7690

7791
User bob = User.getById("bob", true);
@@ -89,6 +103,10 @@ void noAccessForNonAdmin() throws Exception {
89103

90104
assertTrue(mon.isEnabled());
91105

106+
try (ACLContext c = ACL.as(administrator)) {
107+
assertTrue(mon.isEnabled());
108+
}
109+
92110
WebRequest requestReadOnly = new WebRequest(new URI(wc.getContextPath() + "administrativeMonitor/hudsonHomeIsFull").toURL(), HttpMethod.GET);
93111
p = wc.getPage(requestReadOnly);
94112
assertEquals(HttpURLConnection.HTTP_FORBIDDEN, p.getWebResponse().getStatusCode());
@@ -98,14 +116,17 @@ void noAccessForNonAdmin() throws Exception {
98116
request.setRequestParameters(List.of(param));
99117
p = wc.getPage(request);
100118
assertEquals(HttpURLConnection.HTTP_OK, p.getWebResponse().getStatusCode());
101-
assertFalse(mon.isEnabled());
119+
try (ACLContext c = ACL.as(administrator)) {
120+
assertFalse(mon.isEnabled());
121+
}
122+
assertThrows(ElementNotFoundException.class, () -> getForm(mon, administrator));
102123
}
103124

104125
/**
105126
* Gets the warning form.
106127
*/
107-
private HtmlForm getForm(HudsonHomeDiskUsageMonitor mon) throws IOException, SAXException {
108-
HtmlPage p = j.createWebClient().goTo("manage");
128+
private HtmlForm getForm(HudsonHomeDiskUsageMonitor mon, User user) throws IOException, SAXException {
129+
HtmlPage p = j.createWebClient().withBasicApiToken(user).goTo("manage");
109130
return p.getFormByName(mon.id);
110131
}
111132
}

0 commit comments

Comments
 (0)