Skip to content

Commit fcb41b7

Browse files
WadeckWadeck
committed
Adjustment after review
- Wording / tooltips - Removal of badges when there is no needed action - Stop using deprecated ACL.impersonate - Stop using SecurityContextHolder when Jenkins is sufficient - Remove all occurrences of acegi usage in the plugin
1 parent 18bcb49 commit fcb41b7

File tree

4 files changed

+40
-54
lines changed

4 files changed

+40
-54
lines changed

‎src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java‎

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@
2525
package org.jenkinsci.plugins.scriptsecurity.scripts;
2626

2727
import com.google.common.annotations.VisibleForTesting;
28+
import hudson.security.ACLContext;
2829
import hudson.util.HttpResponses;
2930
import jenkins.model.GlobalConfiguration;
3031
import jenkins.model.GlobalConfigurationCategory;
3132
import net.sf.json.JSONArray;
3233
import net.sf.json.JSONObject;
33-
import org.acegisecurity.Authentication;
3434
import org.apache.commons.lang.StringUtils;
3535
import org.jenkinsci.Symbol;
3636
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.AclAwareWhitelist;
@@ -70,15 +70,14 @@
7070
import java.util.function.Consumer;
7171
import java.util.logging.Level;
7272
import java.util.logging.Logger;
73+
import java.util.stream.Stream;
7374
import javax.annotation.CheckForNull;
7475
import javax.annotation.Nonnull;
7576
import javax.annotation.concurrent.GuardedBy;
7677
import javax.servlet.ServletException;
7778

7879
import jenkins.model.Jenkins;
7980
import net.sf.json.JSON;
80-
import org.acegisecurity.context.SecurityContext;
81-
import org.acegisecurity.context.SecurityContextHolder;
8281
import org.jenkinsci.plugins.scriptsecurity.scripts.languages.LanguageHelper;
8382
import org.jenkinsci.plugins.scriptsecurity.scripts.metadata.FullScriptMetadata;
8483
import org.jenkinsci.plugins.scriptsecurity.scripts.metadata.HashAndFullScriptMetadata;
@@ -235,18 +234,12 @@ public void doIndex(StaplerRequest req, StaplerResponse rsp, @QueryParameter("ta
235234
tabInfos[FULL_SCRIPT_PENDING_INDEX].numOfNotification = pendingScripts.size();
236235
tabInfos[FULL_SCRIPT_PENDING_INDEX].primaryColor = true;
237236

238-
tabInfos[FULL_SCRIPT_APPROVED_INDEX].numOfNotification = approvedScriptHashes.size();
239-
240237
tabInfos[SIGNATURE_PENDING_INDEX].numOfNotification = pendingSignatures.size();
241238
tabInfos[SIGNATURE_PENDING_INDEX].primaryColor = true;
242239

243-
tabInfos[SIGNATURE_APPROVED_INDEX].numOfNotification = approvedSignatures.size();
244-
245240
tabInfos[CLASS_PATH_PENDING_INDEX].numOfNotification = pendingClasspathEntries.size();
246241
tabInfos[CLASS_PATH_PENDING_INDEX].primaryColor = true;
247242

248-
tabInfos[CLASS_PATH_APPROVED_INDEX].numOfNotification = approvedClasspathEntries.size();
249-
250243
// will be injected as a variable inside Jelly view
251244
req.setAttribute("activeTab", activeTab);
252245
req.setAttribute("tabInfos", tabInfos);
@@ -971,13 +964,11 @@ public void approveScript(String hash) {
971964

972965
save();
973966
}
974-
SecurityContext orig = ACL.impersonate(ACL.SYSTEM);
975-
try {
967+
968+
try (ACLContext unused = ACL.as(ACL.SYSTEM)) {
976969
for (ApprovalListener listener : ExtensionList.lookup(ApprovalListener.class)) {
977970
listener.onApproved(hash);
978971
}
979-
} finally {
980-
SecurityContextHolder.setContext(orig);
981972
}
982973
}
983974

@@ -1048,15 +1039,13 @@ public void doApprovePendingScripts(@JsonBody AllSelectedHashesModel content) {
10481039

10491040
save();
10501041
}
1051-
SecurityContext orig = ACL.impersonate(ACL.SYSTEM);
1052-
try {
1042+
1043+
try (ACLContext unused = ACL.as(ACL.SYSTEM)) {
10531044
for (ApprovalListener listener : ExtensionList.lookup(ApprovalListener.class)) {
10541045
for (String hash : content.hashes) {
10551046
listener.onApproved(hash);
10561047
}
10571048
}
1058-
} finally {
1059-
SecurityContextHolder.setContext(orig);
10601049
}
10611050
}
10621051

@@ -1249,13 +1238,10 @@ public JSON approveClasspathEntry(String hash) throws IOException {
12491238
}
12501239
}
12511240
if (url != null) {
1252-
SecurityContext orig = ACL.impersonate(ACL.SYSTEM);
1253-
try {
1241+
try (ACLContext unused = ACL.as(ACL.SYSTEM)) {
12541242
for (ApprovalListener listener : ExtensionList.lookup(ApprovalListener.class)) {
12551243
listener.onApprovedClasspathEntry(hash, url);
12561244
}
1257-
} finally {
1258-
SecurityContextHolder.setContext(orig);
12591245
}
12601246
}
12611247
return getClasspathRenderInfo();
@@ -1296,13 +1282,12 @@ public synchronized JSON clearApprovedClasspathEntries() throws IOException {
12961282
* To have an effectively final variable
12971283
*/
12981284
private @CheckForNull String getCurrentUserLogin() {
1299-
String userLogin = null;
1300-
Authentication auth = SecurityContextHolder.getContext().getAuthentication();
1301-
if (auth != null) {
1302-
userLogin = auth.getName();
1303-
}
1304-
1305-
return userLogin;
1285+
// used to remove completely the dependency on Acegi Security
1286+
return Stream.of(Jenkins.getAuthentication())
1287+
.filter(auth -> !auth.equals(Jenkins.ANONYMOUS))
1288+
.findFirst()
1289+
.map(auth -> auth.getName())
1290+
.orElse(null);
13061291
}
13071292

13081293
@Restricted(NoExternalUse.class) // for jelly only

‎src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/tabs/fullScript_approved.jelly‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@
4545
<j:choose>
4646
<j:when test="${metadata.isEmpty()}">
4747
<td colspan="9">
48-
<i title="${%emptyMetadata_tooltip}">${%emptyMetadata}</i>
48+
<i class="info-cursor">
49+
${%emptyMetadata}
50+
</i>
4951
</td>
5052
</j:when>
5153
<j:otherwise>
@@ -61,9 +63,9 @@
6163
</div>
6264
</j:when>
6365
<j:otherwise>
64-
<span title="${%noScriptCodeSinceMetadata_tooltip}" class="info-cursor">
66+
<i title="${%noScriptCodeSinceMetadata_tooltip}" class="info-cursor">
6567
${%noScriptCodeSinceMetadata}
66-
</span>
68+
</i>
6769
</j:otherwise>
6870
</j:choose>
6971
</td>
@@ -75,9 +77,9 @@
7577
<a href="${rootURL}/user/${user}" class="model-link">${user}</a>
7678
</j:when>
7779
<j:otherwise>
78-
<span title="${%noRequesterSinceMetadata_tooltip}" class="info-cursor">
80+
<i title="${%noRequesterSinceMetadata_tooltip}" class="info-cursor">
7981
${%noRequesterSinceMetadata}
80-
</span>
82+
</i>
8183
</j:otherwise>
8284
</j:choose>
8385
</td>
@@ -88,9 +90,9 @@
8890
<a href="${rootURL}/${contextItem.url}" class="model-link">${contextItem.fullDisplayName}</a>
8991
</j:when>
9092
<j:otherwise>
91-
<span title="${%noContextItemSinceMetadata_tooltip}" class="info-cursor">
93+
<i title="${%noContextItemSinceMetadata_tooltip}" class="info-cursor">
9294
${%noContextItemSinceMetadata}
93-
</span>
95+
</i>
9496
</j:otherwise>
9597
</j:choose>
9698
</td>
@@ -101,9 +103,9 @@
101103
<span title="${%usageCount_tooltip}" class="info-cursor">${usageCount}</span>
102104
</j:when>
103105
<j:otherwise>
104-
<span title="${%noUsageCountSinceMetadata_tooltip}" class="info-cursor">
106+
<i title="${%noUsageCountSinceMetadata_tooltip}" class="info-cursor">
105107
${%noUsageCountSinceMetadata}
106-
</span>
108+
</i>
107109
</j:otherwise>
108110
</j:choose>
109111
</td>
@@ -114,9 +116,9 @@
114116
<i:formatDate value="${lastTimeUsed}" type="both" dateStyle="medium" timeStyle="medium"/>
115117
</j:when>
116118
<j:otherwise>
117-
<span title="${%notUsedSinceMetadata_tooltip}" class="info-cursor">
119+
<i title="${%notUsedSinceMetadata_tooltip}" class="info-cursor">
118120
${%notUsedSinceMetadata}
119-
</span>
121+
</i>
120122
</j:otherwise>
121123
</j:choose>
122124
</td>
@@ -134,9 +136,9 @@
134136
<i:formatDate value="${lastApprovalTime}" type="both" dateStyle="medium" timeStyle="medium" />
135137
</j:when>
136138
<j:otherwise>
137-
<span title="${%notApprovedSinceMetadata_tooltip}" class="info-cursor">
139+
<i title="${%notApprovedSinceMetadata_tooltip}" class="info-cursor">
138140
${%notApprovedSinceMetadata}
139-
</span>
141+
</i>
140142
</j:otherwise>
141143
</j:choose>
142144
</td>
@@ -147,9 +149,9 @@
147149
<a href="${rootURL}/user/${lastKnownApproverLogin}" class="model-link">${lastKnownApproverLogin}</a>
148150
</j:when>
149151
<j:otherwise>
150-
<span title="${%noKnownApproverSinceMetadata_tooltip}" class="info-cursor">
152+
<i title="${%noKnownApproverSinceMetadata_tooltip}" class="info-cursor">
151153
${%noKnownApproverSinceMetadata}
152-
</span>
154+
</i>
153155
</j:otherwise>
154156
</j:choose>
155157
</td>

‎src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/tabs/fullScript_approved.properties‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ metadataGatheringDisabled=Metadata gathering disabled. \
99
approvedExpandCode=Code
1010
approvedLanguage=Language
1111
usageCount=# of uses
12-
usageCount_tooltip=Uses since the introduction of metadata
12+
usageCount_tooltip=How often this script was executed. Only executions since Script Security plugin was updated to version 1.75.
1313
lastTimeUsed=Date of last use
1414
wasPreapproved_header_tooltip=The presence of a green tick means the script was approved during the configuration. \
1515
Otherwise it was created by a user without the permission to run scripts and was then approved using this page.
@@ -21,12 +21,10 @@ lastKnownApproverLogin=Last approver
2121
noRequesterSinceMetadata=N/A
2222
noRequesterSinceMetadata_tooltip=The requester was not recorded. \
2323
It could be because the request was created before the metadata was introduced or the code calling this script did not provide user information.
24-
noRequestApprovalTimeSinceMetadata=N/A
25-
noRequestApprovalTimeSinceMetadata_tooltip=It was requested before the introduction of metadata.
2624
noContextItemSinceMetadata=N/A
2725
noContextItemSinceMetadata_tooltip=The context item was not provided by the code asking for approval.
2826

29-
emptyMetadata=There is no metadata for this approval
27+
emptyMetadata=There is no metadata for this approval.
3028
emptyMetadata_tooltip=If the script is used after the metadata introduction, some metadata will be displayed here. \
3129
It means that an unused approval will not have metadata and could be reasonably removed after a certain period of time.
3230
noScriptCodeSinceMetadata=N/A
@@ -45,9 +43,10 @@ noKnownApproverSinceMetadata=N/A
4543
noKnownApproverSinceMetadata_tooltip=It was not approved since the introduction of metadata.
4644

4745
revokeApproval=Remove selected
48-
revokeApproval_tooltip=Remove an approved script can have an impact on the jobs using it. \
49-
The next time an attempt to execute the script is made, a new approval will be required.
50-
revokeApproval_confirm=Really delete all selected approvals? Any existing scripts will need to be requeued and reapproved.
46+
revokeApproval_tooltip=Removing a script from this list will revoke approval for its execution. \
47+
The next attempt to execute the script will fail, and the script will be added to the list of scripts requiring approval.
48+
revokeApproval_confirm=Really revoke approvals for all selected scripts? \
49+
All scripts not using the sandbox will need to be approved again before they can be executed successfully, and will fail until they are.
5150
approvedNoSelectedItemError=Please select at least one line.
5251

5352
approvedExpand=show code

‎src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/tabs/fullScript_pending.jelly‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@
4141
<a href="${rootURL}/user/${user}" class="model-link">${user}</a>
4242
</j:when>
4343
<j:otherwise>
44-
<span title="${%noRequesterSinceMetadata_tooltip}" class="info-cursor">
44+
<i title="${%noRequesterSinceMetadata_tooltip}" class="info-cursor">
4545
${%noRequesterSinceMetadata}
46-
</span>
46+
</i>
4747
</j:otherwise>
4848
</j:choose>
4949
</td>
@@ -54,9 +54,9 @@
5454
<a href="${rootURL}/${contextItem.url}" class="model-link">${contextItem.fullDisplayName}</a>
5555
</j:when>
5656
<j:otherwise>
57-
<span title="${%noContextItemSinceMetadata_tooltip}" class="info-cursor">
57+
<i title="${%noContextItemSinceMetadata_tooltip}" class="info-cursor">
5858
${%noContextItemSinceMetadata}
59-
</span>
59+
</i>
6060
</j:otherwise>
6161
</j:choose>
6262
</td>

0 commit comments

Comments
 (0)