Skip to content

Commit 50f5c5c

Browse files
committed
feat: improve boolean reability
1 parent 8507a9c commit 50f5c5c

File tree

1 file changed

+20
-16
lines changed

1 file changed

+20
-16
lines changed

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

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -618,28 +618,32 @@ synchronized boolean isEmpty() {
618618
* @param language the language in which it is written
619619
* @param context any additional information about how where or by whom this is being configured
620620
* @param approveIfAdmin indicates whether script should be approved if current user has admin permissions
621-
* @param ignoreAdmin indicates whether auto approval should be ignored, regardless of any configurations.
621+
* @param ignoreAdmin indicates whether an admin's ability to approve a script without visiting the script approval site should be ignored, regardless of any configurations.
622622
* @return {@code script}, for convenience
623623
*/
624624
public synchronized String configuring(@NonNull String script, @NonNull Language language, @NonNull ApprovalContext context, boolean approveIfAdmin, boolean ignoreAdmin) {
625625
final ConversionCheckResult result = checkAndConvertApprovedScript(script, language);
626-
if (!result.approved) {
627-
if (!Jenkins.get().isUseSecurity() ||
628-
(ALLOW_ADMIN_APPROVAL_ENABLED &&
629-
((Jenkins.getAuthentication2() != ACL.SYSTEM2 && Jenkins.get().hasPermission(Jenkins.ADMINISTER))
630-
&& (ADMIN_AUTO_APPROVAL_ENABLED || approveIfAdmin) && !ignoreAdmin))) {
631-
approvedScriptHashes.add(result.newHash);
632-
//Pending scripts are not stored with a precalculated hash, so no need to remove any old hashes
633-
removePendingScript(result.newHash);
634-
} else {
635-
String key = context.getKey();
636-
if (key != null) {
637-
pendingScripts.removeIf(pendingScript -> key.equals(pendingScript.getContext().getKey()));
638-
}
639-
pendingScripts.add(new PendingScript(script, language, context));
626+
if (result.approved) {
627+
return script;
628+
}
629+
// Security is disabled globally.
630+
boolean securityIsDisabled = !Jenkins.get().isUseSecurity();
631+
// Has to be an actual user and the user must be admin. System-triggered jobs should not auto-approve. (I guess that is the reasonf or this boolean?)
632+
boolean isAdminUser = Jenkins.getAuthentication2() != ACL.SYSTEM2 && Jenkins.get().hasPermission(Jenkins.ADMINISTER);
633+
boolean implicitAdminApproval = ADMIN_AUTO_APPROVAL_ENABLED || approveIfAdmin;
634+
if (securityIsDisabled ||
635+
(ALLOW_ADMIN_APPROVAL_ENABLED && isAdminUser && implicitAdminApproval && !ignoreAdmin)) {
636+
approvedScriptHashes.add(result.newHash);
637+
//Pending scripts are not stored with a precalculated hash, so no need to remove any old hashes
638+
removePendingScript(result.newHash);
639+
} else {
640+
String key = context.getKey();
641+
if (key != null) {
642+
pendingScripts.removeIf(pendingScript -> key.equals(pendingScript.getContext().getKey()));
640643
}
641-
save();
644+
pendingScripts.add(new PendingScript(script, language, context));
642645
}
646+
save();
643647
return script;
644648
}
645649

0 commit comments

Comments
 (0)